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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +reftable.lockTimeout::
> +	Whenever the reftable backend appends a new table to the stack, it has
> +	to lock the central "tables.list" file before updating it. This config
> +	controls how long the process will wait to acquire the lock in case
> +	another process has already acquired it. Value 0 means not to retry at
> +	all; -1 means to try indefinitely. Default is 100 (i.e., retry for
> +	100ms).

Existing timeout knobs are in a hierarchy that is too wide
(i.e. core.*timeout) and this fixes the mistake by placing the name
in a lot more appropriate name (i.e. reftable.*timeout).  If I were
designing the system from scratch, I would probably place all of
them in "refs.*timeout", but I do not think it is worth extra
engineering effort to rename them and pay the transition cost.

> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 1c4b19e737f..ca281e39a29 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -256,6 +256,13 @@ 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")) {
> +		int64_t lock_timeout = git_config_int64(var, value, ctx->kvi);
> +		if (lock_timeout > LONG_MAX)
> +			die("reftable lock timeout cannot exceed %"PRIdMAX, (intmax_t)LONG_MAX);
> +		if (lock_timeout < 0 && lock_timeout != -1)
> +			die("reftable lock timeout does not support negative values other than -1");
> +		opts->lock_timeout_ms = lock_timeout;

Existing lock timeouts this models after seems to consider a
platform native "int" is good enough size to represent the timeout
value in milliseconds, but the eventual user of this value in the
lockfile API expects a long, so lock_timeout_ms being long is fine.

Perhaps #leftoverbits to straighten out the types used for the other
two timeout configuration variables.