Re: [PATCH v4 27/28] reftable: fixup for new base topic 2/3
- From
Han-Wen Nienhuys <hanwen@google.com>
- Date
- Aug 30, 2021, 12:32 UTC
- Message-ID
- <CAFQ2z_MnJr58Cu970Be7SXE1jFkgOGThQ0Geq2+kNihJE3_BvQ@mail.gmail.com>
- In-Reply-To
- <patch-v4-27.28-c4f9fb42d9e-20210823T120208Z-avarab@gmail.com>
On Mon, Aug 23, 2021 at 2:13 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 12 quoted lines
> > Since my "refs API: remove OID argument to reflog_expire()" we don't > have the "oid" as part of the reflog_expire() signature. Instead the > reflog_expire() should pass the OID of the tip of the "locked" ref to > the prepare_fn(). > > In files_reflog_expire() we do that by getting the OID from > lock_ref_oid_basic(). I'm assuming (but am not familiar enough with > reftable...) that by the time we get here we've got a locked ref > already in some way, so let's just use > refs_resolve_ref_unsafe_with_errno() to lookup the current OID of that > presumably-locked ref.
I quickly looked at the files code, but I don't understand why the OID needs to be passed-in (before your refactoring): in builtin/reflog.c (before), the current OID is read, with any protection. This means that its value can't be trusted.
After your refactoring, you lock the ref. I guess in the files backend this protects against non-atomic update of (ref, reflog) racing with a concurrent reflog expiry? In reftable, the (ref,reflog) update is atomic, so there is no need for locking to properly sequence operations.
-- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado