Re: [PATCH] logging branch deletion to help recovering from mistakes

Nguyen Thai Ngoc Duy [off-list ref] writes:

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

On Tue, Dec 7, 2010 at 1:28 PM, Junio C Hamano [off-list ref] wrote:

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

I think you have thought of this. What's wrong with keeping reflog
when a branch is removed and appending "delete" line to the said
reflog? I don't know how reflogs are managed, but those reflogs
without associated branch will (or should) be cleaned when they are
expired.

I stick with this idea because I also want to archive old branches and
am thinking those reflogs ending with "archive" line will be kept
forever, or until I feel like digging up them again.
-- 
Duy

Nguyen Thai Ngoc Duy venit, vidit, dixit 07.12.2010 12:37:

On Tue, Dec 7, 2010 at 1:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

I think you have thought of this. What's wrong with keeping reflog
when a branch is removed and appending "delete" line to the said
reflog? I don't know how reflogs are managed, but those reflogs
without associated branch will (or should) be cleaned when they are
expired.

The problem is the following:

Say, you delete a branch and its reflog is kept (with a "delete" line
appended).

Then you create a new branch under the same name. What is supposed to
happen to the reflog? If you simply append, then old (unrelated) entries
will not expire through the imagined "expire branch reflogs" mechanism.

Now, you rename that branch. We should really split the reflog in two
now, keeping the old name for the old parts and moving only the newer
parts to the reflog with the new name.

This is all workable in principle but hints at a design flaw.

Maybe it's easier to teach "git reflog" about "DELETED_REFS"?

Michael

Nguyen Thai Ngoc Duy [off-list ref] writes:

On Tue, Dec 7, 2010 at 1:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

I think you have thought of this. What's wrong with keeping reflog
when a branch is removed and appending "delete" line to the said
reflog? I don't know how reflogs are managed, but those reflogs
without associated branch will (or should) be cleaned when they are
expired.

I stick with this idea because I also want to archive old branches and
am thinking those reflogs ending with "archive" line will be kept
forever, or until I feel like digging up them again.

The problem with this idea is deleting branch 'foo' and creating 'foo/bar',
or deleting branch 'foo/bar' and creating branch 'foo'.  Old reflog with
"delete" line would block creating reflog for new branch.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

On Tue, Dec 7, 2010 at 10:25 PM, Michael J Gruber
[off-list ref] wrote:

Nguyen Thai Ngoc Duy venit, vidit, dixit 07.12.2010 12:37:

quoted

On Tue, Dec 7, 2010 at 1:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

I think you have thought of this. What's wrong with keeping reflog
when a branch is removed and appending "delete" line to the said
reflog? I don't know how reflogs are managed, but those reflogs
without associated branch will (or should) be cleaned when they are
expired.

The problem is the following:

Say, you delete a branch and its reflog is kept (with a "delete" line
appended).

Then you create a new branch under the same name. What is supposed to
happen to the reflog? If you simply append, then old (unrelated) entries
will not expire through the imagined "expire branch reflogs" mechanism.

Now, you rename that branch. We should really split the reflog in two
now, keeping the old name for the old parts and moving only the newer
parts to the reflog with the new name.

I don't see any problems with that. If I happen to create a branch
with the same name, most of the time, there is something related,
unless for very generic names like "tmp". We can always notify users
about the accident resurrection of an old branch at branch creation,
so they can remove the old reflog if they want.
-- 
Duy

On Tue, Dec 7, 2010 at 11:22 PM, Jakub Narebski [off-list ref] wrote:

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 1:28 PM, Junio C Hamano [off-list ref] wrote:

quoted

Nguyen Thai Ngoc Duy [off-list ref] writes:

quoted

On Tue, Dec 7, 2010 at 4:16 AM, Junio C Hamano [off-list ref] wrote:

quoted

+#define BRANCH_DELETION_LOG "DELETED-REFS"
+

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

I think you have thought of this. What's wrong with keeping reflog
when a branch is removed and appending "delete" line to the said
reflog? I don't know how reflogs are managed, but those reflogs
without associated branch will (or should) be cleaned when they are
expired.

I stick with this idea because I also want to archive old branches and
am thinking those reflogs ending with "archive" line will be kept
forever, or until I feel like digging up them again.

The problem with this idea is deleting branch 'foo' and creating 'foo/bar',
or deleting branch 'foo/bar' and creating branch 'foo'.  Old reflog with
"delete" line would block creating reflog for new branch.

Thanks. That makes sense.
-- 
Duy

On Mon, Dec 06, 2010 at 10:28:53PM -0800, Junio C Hamano wrote:

quoted

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

Yeah, I think this is not _quite_ what people want in this area. A base
requirement from past discussions, I think, is that the whole reflog of
the deleted branch be saved rather than just the tip. And then "reflog
show" would make a lot more sense on such saved reflogs.

I'm not sure in practice how important that distinction is, as we are
not saving deleted branch reflogs _at all_ right now, so the
requirements are mostly speculation at this point.

The most recent discussion I recall is this one:

  http://thread.gmane.org/gmane.comp.version-control.git/144250/focus=145353

where the general idea was to just keep deleted reflogs around, append
to them if the branch was recreated, and use a consistent renaming
scheme to avoid D/F naming conflicts (e.g., "foo" is a deleted ref, and
you create "foo/bar").

-Peff

On Tue, Dec 7, 2010 at 9:06 AM, Jeff King [off-list ref] wrote:

On Mon, Dec 06, 2010 at 10:28:53PM -0800, Junio C Hamano wrote:

quoted

quoted

Should this special log be mentioned in git-update-ref.txt or
gitrepository-layout.txt?

Perhaps, but I wasn't sure if this patch itself is a good idea to begin
with.  Not the problem it tries to solve, but its approach.

For example, this cannot be shown with "reflog show" or "log -g" due to
the way these frontends locate the reflog file to read (the logic wants to
have an underlying ref).

Yeah, I think this is not _quite_ what people want in this area. A base
requirement from past discussions, I think, is that the whole reflog of
the deleted branch be saved rather than just the tip. And then "reflog
show" would make a lot more sense on such saved reflogs.

Yup, that's what I recall too, folks (including myself) want to
save the reflog of the deleted branch, so it can be recovered if
the branch itself were to be recovered with an --undelete option.

I'm not sure in practice how important that distinction is, as we are
not saving deleted branch reflogs _at all_ right now, so the
requirements are mostly speculation at this point.

The most recent discussion I recall is this one:

 http://thread.gmane.org/gmane.comp.version-control.git/144250/focus=145353

where the general idea was to just keep deleted reflogs around, append
to them if the branch was recreated, and use a consistent renaming
scheme to avoid D/F naming conflicts (e.g., "foo" is a deleted ref, and
you create "foo/bar").

Per check-ref-format, ref names cannot contain two dots.  We could
archive ref logs by renaming them, $GIT_DIR/logs/refs/heads/foo
becomes $GIT_DIR/logs/refs/heads/foo..deleted-1.  If foo is created
and deleted again, it becomes foo..deleted-2.

This still causes problems for git reflog show / git log -g because
they want a current ref to enumerate the log of.


A different approach might be to have $GIT_DIR/logs/refs/REF_ATTIC,
and special case that in git reflog show / git log -g.  When a
ref is deleted, append its entire log onto REF_ATTIC, between two
specially formatted marker lines.  When recovering a branch, copy
out the region from the REF_ATTIC log.

-- 
Shawn.

On Tue, Dec 07, 2010 at 10:14:19AM -0800, Shawn O. Pearce wrote:

Per check-ref-format, ref names cannot contain two dots.  We could
archive ref logs by renaming them, $GIT_DIR/logs/refs/heads/foo
becomes $GIT_DIR/logs/refs/heads/foo..deleted-1.  If foo is created
and deleted again, it becomes foo..deleted-2.

This still causes problems for git reflog show / git log -g because
they want a current ref to enumerate the log of.

That seems reasonable to me. The "reflog show" limitation is just a
matter of a simple code fix, though, isn't it? Is there a good reason
for this restriction to exist? And even if there is, it would be simple
to special case it for ..deleted-* branches.

A different approach might be to have $GIT_DIR/logs/refs/REF_ATTIC,
and special case that in git reflog show / git log -g.  When a
ref is deleted, append its entire log onto REF_ATTIC, between two
specially formatted marker lines.  When recovering a branch, copy
out the region from the REF_ATTIC log.

That seems a lot less efficient, as we have to linearly search all of
REF_ATTIC to get:

  1. the reflog for one deleted branch

  2. the list of deleted branches

Neither of those is probably particularly performance critical, but it
just seems like keeping the logs in files indexed by the original ref
names is a more natural fit.

-Peff

Jeff King [off-list ref] wrote:

On Tue, Dec 07, 2010 at 10:14:19AM -0800, Shawn O. Pearce wrote:

quoted

Per check-ref-format, ref names cannot contain two dots.  We could
archive ref logs by renaming them, $GIT_DIR/logs/refs/heads/foo
becomes $GIT_DIR/logs/refs/heads/foo..deleted-1.  If foo is created
and deleted again, it becomes foo..deleted-2.

...

quoted

A different approach might be to have $GIT_DIR/logs/refs/REF_ATTIC,

That seems a lot less efficient, as we have to linearly search all of
REF_ATTIC to get:

  1. the reflog for one deleted branch

  2. the list of deleted branches

Neither of those is probably particularly performance critical, but it
just seems like keeping the logs in files indexed by the original ref
names is a more natural fit.

Yea, I'm leaning more towards the foo..deleted-n idea too, for the
same reasons.  It also makes it easier to GC a deleted branch's
reflog, we can examine the last record's timestamp in a reasonable
time bound and unlink the log if its really freaking old.

-- 
Shawn.

On Tue, Dec 07, 2010 at 10:23:42AM -0800, Shawn O. Pearce wrote:

Yea, I'm leaning more towards the foo..deleted-n idea too, for the
same reasons.  It also makes it easier to GC a deleted branch's
reflog, we can examine the last record's timestamp in a reasonable
time bound and unlink the log if its really freaking old.

Do we need to actually do that? Shouldn't the entries in the reflog get
expired as part of the regular reflog gc? In that case, we would just
delete the file when it had zero entries.

-Peff

On Tue, Dec 07, 2010 at 10:37:39AM -0800, Shawn O. Pearce wrote:

Yes, you are right.  We should instead let the normal reflog expire
action do its work here, and delete the empty log file when it is
finally empty.

I guess we also need repack and prune to enumerate these deleted
reflogs and retain the objects their records point to.

Definitely. I sort of assumed all of those things just traversed
.git/logs blindly without regard to whether there was a ref, which would
handle this automagically. But maybe that is not the case.

Is there a reason to require that each log is specifically tied to a
ref?

-Peff

Jeff King [off-list ref] wrote:

On Tue, Dec 07, 2010 at 10:37:39AM -0800, Shawn O. Pearce wrote:

quoted

Yes, you are right.  We should instead let the normal reflog expire
action do its work here, and delete the empty log file when it is
finally empty.

I guess we also need repack and prune to enumerate these deleted
reflogs and retain the objects their records point to.

Definitely. I sort of assumed all of those things just traversed
.git/logs blindly without regard to whether there was a ref, which would
handle this automagically. But maybe that is not the case.

I think those enumerate the logs of refs that are also being
traversed.  Which means we would need to add new logic to enumerate
the deleted reflogs.
 

Is there a reason to require that each log is specifically tied to a
ref?

Historical bad assumptions?

I mean, no, there really isn't a good reason that each log is
tied to a ref.  Its probably reasonable to just enumerate the logs
directory separate from the refs directory enumeration.  Its just
some more code.  Right now we discover logs by just relying on the
ref directory traversal code.

-- 
Shawn.

Jeff King [off-list ref] wrote:

On Tue, Dec 07, 2010 at 10:23:42AM -0800, Shawn O. Pearce wrote:

quoted

Yea, I'm leaning more towards the foo..deleted-n idea too, for the
same reasons.  It also makes it easier to GC a deleted branch's
reflog, we can examine the last record's timestamp in a reasonable
time bound and unlink the log if its really freaking old.

Do we need to actually do that? Shouldn't the entries in the reflog get
expired as part of the regular reflog gc? In that case, we would just
delete the file when it had zero entries.

Yes, you are right.  We should instead let the normal reflog expire
action do its work here, and delete the empty log file when it is
finally empty.

I guess we also need repack and prune to enumerate these deleted
reflogs and retain the objects their records point to.

-- 
Shawn.