Re: [PATCH v2] config: retry acquiring config.lock, configurable via core.configLockTimeout

> Joerg Thalheim <joerg@thalheim.io> writes:
> 
> > +/*
> > + * How long to retry acquiring config.lock when another process holds
> > + * it. Default matches core.packedRefsTimeout; override via
> > + * core.configLockTimeout.
> > + */
> > +static long config_lock_timeout_ms(struct repository *r)
> > +{
> > +	static int configured;
> > +	static int timeout_ms = 1000;
> > +
> > +	if (!configured) {
> > +		repo_config_get_int(r, "core.configlocktimeout", &timeout_ms);
> > +		configured = 1;
> > +	}
> > +
> > +	return timeout_ms;
> > +}
> 
> The above design means whichever repository happens to be passed for
> the first time as "r" to this call will fix the return value from
> the function for the rest of the system, meaning that the lock timeout
> is a per-process property and the repository parameter passed to the
> function does not matter all that much.
> 
> It may make sense to admit that this is not a per-repository
> property (due to the use of local caching), have the function take
> no parameter and use the_repository to the config_get call.  That
> would make the intention more clear.
> 
> Of course the other end of the spectrum is to get rid of the
> "configured" caching here, and ask the config system to make a
> hashtable look-up every time the function is called.  That will keep
> the lock timeout per-repository, which is closer to what the current
> function signature suggests.
> 
> I dunno.  My gut feeling is that there aren't valid reasons why you
> would want to specifically set different timeout values per
> repository, so the simplicity of using the_repository (i.e. the
> primary repository instance this process deals with) sounds like a
> better way to go.