Subject: Re: [PATCH v2 1/3] refs/reftable: introduce "reftable.lockTimeout"

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 18 Sep 2024 11:39:05 +0200

On Wed, Sep 18, 2024 at 07:22:28PM +1000, James Liu wrote:
> On Wed Sep 18, 2024 at 2:32 PM AEST, Patrick Steinhardt wrote:
> > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> > index 1c4b19e737f..e90ddfb98dd 100644
> > --- a/refs/reftable-backend.c
> > +++ b/refs/reftable-backend.c
> > @@ -256,6 +256,9 @@ static int reftable_be_config(const char *var, const char *value,
> >  		if (factor > UINT8_MAX)
> >  			die("reftable geometric factor cannot exceed %u", (unsigned)UINT8_MAX);
> >  		opts->auto_compaction_factor = factor;
> > +	} else if (!strcmp(var, "reftable.locktimeout")) {
> > +		unsigned long lock_timeout = git_config_ulong(var, value, ctx->kvi);
> > +		opts->lock_timeout_ms = lock_timeout;
> >  	}
> 
> Do we need to support the `0` and `-1` values that are possible for
> the "core.filesRefLockTimeout" and "core.packedRefsTimeout" timeouts
> here as well?

We already handle `0`, which is provided by the underlying lockfile
interface. But we don't handle `-1` yet. I guess wiring it up does make
sense indeed, even if it is just to be consistent with the other
timeouts.

Patrick