Patrick Steinhardt <ps@xxxxxx> 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.
