Thread (3 messages) 3 messages, 3 authors, 2026-01-09

Re: [PATCH] fsck: snapshot default refs before object walk

From: Elijah Newren <hidden>
Date: 2026-01-06 23:19:34

On Mon, Dec 29, 2025 at 4:46 PM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted
This problem doesn't occur when refs are specified on the command line
for us to check, since we use those specified refs for both walking and
checking.  Using the same refs for walking and checking seems to just
make sense, so modify the existing code to do the same when refs aren't
specified.
Excellent analysis and good approach.
quoted
Snapshot the refs at the beginning, and also ignore all
reflog entries since the time of our snapshot (while this technically
means we could ignore a reflog entry created before the fsck process
if the local clock is weird, since reflogs are local-only there are not
concerns about differences between clocks on different machines).
Repository on a network filesystem being accessed by hosts with
broken clock?
Oh, indeed.
I do not think our reflog API has (1) give me some token to mark
your current state (2) here is the token you gave me earlier, now
iterate and yield entries but ignore entries added after you gave me
that token, so going by the reflog timestamp is probably the best we
could do.  Any approach may get confused when the user tries to be
cute and issues "reflog delete" or "reflog expire" in the middle
anyway, I suspect ;-)
quoted
While worries about live updates while running fsck is likely of most
interest for forge operators, it will likely also benefit those with
automated jobs (such as git maintenance) or even casual users who want
to do other work in their clone while fsck is running.
Great.  Will queue.  Thanks.
quoted
@@ -509,6 +510,9 @@ static int fsck_handle_reflog_ent(const char *refname,
                                timestamp_t timestamp, int tz UNUSED,
                                const char *message UNUSED, void *cb_data UNUSED)
 {
+     if (now && timestamp > now)
+             return 0;
+
      if (verbose)
              fprintf_ln(stderr, _("Checking reflog %s->%s"),
                         oid_to_hex(ooid), oid_to_hex(noid));
@@ -567,14 +571,53 @@ static int fsck_head_link(const char *head_ref_name,
                        const char **head_points_at,
                        struct object_id *head_oid);

-static void get_default_heads(void)
+struct ref_snapshot {
+     size_t nr;
+     size_t name_alloc;
+     size_t oid_alloc;
+     char **refname;
+     struct object_id *oid;
+};
This data structure is somewhat unexpected.  Instead of a struct
that holds two arrays, I would have rather expected an array of
"struct { refname, oid }", with the possiblity to add a "token to
mark the latest reflog entry" to the mix I alluded to earlier when
such an API function materializes.
Yeah, that makes sense.  It'll mean that there won't be anything left
of Matthew's original patch that I was trying to upstream (especially
with the further changes Peff highlighted elsewhere in this thread),
but I can just take the authorship and note Matthew's contribution in
a trailer.
[Footnote]

We could call refs_for_each_reflog_ent_reverse(), grab the
parameters that each_reflog_ent_fn receives as that "token" for the
latest reflog entry and stop.  That way, we will learn the value of
<old,new,committer,timestamp,tz,msg>, which should be a robust
enough unique key.

After that when iterating over the reflog, we know we should stop
after processing the reflog entry that holds the recorded value.
Interesting.  The global timestamp for reflogs seems good enough for
me (network filesystems with a broken clock feel niche to me), but I
can leave a TODO in the code for those that want to pursue improving
the reflog handling further.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help