Re: [PATCH v5 06/16] refs/files: batch refname availability checks for normal transactions

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 12, 2025, 15:36 UTC
Message-ID: <Z9GqCsobVxnFI-rc@pks.im>
In-Reply-To: <Z9GFBl5AIQZGyI-9@ArchLinux>

On Wed, Mar 12, 2025 at 08:58:46PM +0800, shejialuo wrote:

Show 22 quoted lines

> On Thu, Mar 06, 2025 at 04:08:37PM +0100, Patrick Steinhardt wrote:
> > @@ -2930,6 +2929,26 @@ static int files_transaction_prepare(struct ref_store *ref_store,
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * Verify that none of the loose reference that we're about to write
> > +	 * conflict with any existing packed references. Ideally, we'd do this
> > +	 * check after the packed-refs are locked so that the file cannot
> > +	 * change underneath our feet. But introducing such a lock now would
> > +	 * probably do more harm than good as users rely on there not being a
> > +	 * global lock with the "files" backend.
> > +	 *
> > +	 * Another alternative would be to do the check after the (optional)
> > +	 * lock, but that would extend the time we spend in the globally-locked
> > +	 * state.
> > +	 *
> > +	 * So instead, we accept the race for now.
> > +	 */
> 
> I am curious why we don't sort the `refnames_to_check` here. What is the
> difference between the reftable backend and files backend?

We do sort because we use `string_list_insert()` here. But in fact, we don't have to sort at all, so I'll stop sorting in the preceding commit and convert this callsite here to use `string_list_append()` instead.

Patrick