From: Maciej Ciemborowicz Date: Thu, 24 Sep 2026 19:43:42 GMT Subject: Re: [PATCH v5 1/3] refs: allow callers to supply old OIDs for batch deletion Message-ID: In-Reply-To: On Thu, Sep 24, 2026 at 12:05 PM Karthik Nayak wrote: > This also completes the conversion that was missed when > batched transaction failure support was introduced. Agreed, that sentence is unclear. I will try to remove the batch deletion API changes, along with this sentence and the caller-specific approach. > This change is totally different from the rest of the commit, I think it > should be a precursor with adequate explanation regarding why this is > done and why that's okay. > > I'm also still of the opinion that this shouldn't be done. I will try to leave ref_transaction_delete() and its null_oid BUG() unchanged. Instead, store the value observed for the hook in separate fields that do not set REF_HAVE_OLD and therefore do not constrain the transaction. > I'm assuming failures is to count the number of refs which failed, > wouldn't failed_refs->nr give us the same result? failed_refs is optional, which is why a separate counter is needed. If I move the solution into the common transaction layer, both the failed_refs parameter and the counter should no longer be needed. > Nit: we could inline the size_t here. I will fix this in the next version. Thanks for the review. - Maciej Ciemborowicz