git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 17, 2024, 13:43 UTC
Message-ID
<ca3eab99f7ef86d1b7a5b4d4bdb8d2b0a55566e1.1726578382.git.ps@pks.im>
In-Reply-To
<cover.1726578382.git.ps@pks.im>

When multiple concurrent processes try to update references in a repository they may try to lock the same lockfiles. This can happen even when the updates are non-conflicting and can both be applied, so it doesn't always make sense to abort the transaction immediately. Both the "loose" and "packed" backends thus have a grace period that they wait for the lock to be released that can be controlled via the config values "core.filesRefLockTimeout" and "core.packedRefsTimeout", respectively.

The reftable backend doesn't have such a setting yet and instead fails immediately when it sees such a lock. But the exact same concepts apply here as they do apply to the other backends.

Introduce a new "reftable.lockTimeout" config that controls how long we may wait for a "tables.list" lock to be released. The default value of this config is 100ms, which is the same default as we have it for the "loose" backend.

Note that even though we also lock individual tables, this config really only applies to the "tables.list" file. This is because individual tables are only ever locked when we already hold the "tables.list" lock during compaction. When we observe such a lock we in fact do not want to compact the table at all because it is already in the process of being compacted by a concurrent process. So applying the same timeout here would not make any sense and only delay progress.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/reftable.txt |  7 +++++++
 refs/reftable-backend.c           |  4 ++++
 reftable/reftable-writer.h        |  8 ++++++++
 reftable/stack.c                  | 18 ++++++++++++------
 t/t0610-reftable-basics.sh        | 27 +++++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/Documentation/config/reftable.txt b/Documentation/config/reftable.txt
index 05157279778..6c9449d0232 100644
--- a/Documentation/config/reftable.txt
+++ b/Documentation/config/reftable.txt
@@ -46,3 +46,10 @@ reftable.geometricFactor::
 By default, the geometric sequence uses a factor of 2, meaning that for any
 table, the next-biggest table must at least be twice as big. A maximum factor
 of 256 is supported.
+
+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. Default is 1000 (i.e., retry
+	for 1 second).
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;
 	}
 
 	return 0;
@@ -281,6 +284,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,
 	refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
 	refs->write_options.disable_auto_compact =
 		!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
+	refs->write_options.lock_timeout_ms = 100;
 
 	git_config(reftable_be_config, &refs->write_options);
 
diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h
index 189b1f4144f..d5f2446220b 100644
--- a/reftable/reftable-writer.h
+++ b/reftable/reftable-writer.h
@@ -51,6 +51,14 @@ struct reftable_write_options {
 	 * tables to compact. Defaults to 2 if unset.
 	 */
 	uint8_t auto_compaction_factor;
+
+	/*
+	 * The number of milliseconds to wait when trying to lock "tables.list".
+	 * Note that this does not apply to locking individual tables, as these
+	 * should only ever be locked when already holding the "tables.list"
+	 * lock.
+	 */
+	unsigned lock_timeout_ms;
 };
 
 /* reftable_block_stats holds statistics for a single block type */
diff --git a/reftable/stack.c b/reftable/stack.c
index ce0a35216ba..5ccad2cff31 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -603,8 +603,10 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
 
 	add->stack = st;
 
-	err = hold_lock_file_for_update(&add->tables_list_lock, st->list_file,
-					LOCK_NO_DEREF);
+	err = hold_lock_file_for_update_timeout(&add->tables_list_lock,
+						st->list_file,
+						LOCK_NO_DEREF,
+						st->opts.lock_timeout_ms);
 	if (err < 0) {
 		if (errno == EEXIST) {
 			err = REFTABLE_LOCK_ERROR;
@@ -1056,8 +1058,10 @@ static int stack_compact_range(struct reftable_stack *st,
 	 * Hold the lock so that we can read "tables.list" and lock all tables
 	 * which are part of the user-specified range.
 	 */
-	err = hold_lock_file_for_update(&tables_list_lock, st->list_file,
-					LOCK_NO_DEREF);
+	err = hold_lock_file_for_update_timeout(&tables_list_lock,
+						st->list_file,
+						LOCK_NO_DEREF,
+						st->opts.lock_timeout_ms);
 	if (err < 0) {
 		if (errno == EEXIST)
 			err = REFTABLE_LOCK_ERROR;
@@ -1156,8 +1160,10 @@ static int stack_compact_range(struct reftable_stack *st,
 	 * "tables.list". We'll then replace the compacted range of tables with
 	 * the new table.
 	 */
-	err = hold_lock_file_for_update(&tables_list_lock, st->list_file,
-					LOCK_NO_DEREF);
+	err = hold_lock_file_for_update_timeout(&tables_list_lock,
+						st->list_file,
+						LOCK_NO_DEREF,
+						st->opts.lock_timeout_ms);
 	if (err < 0) {
 		if (errno == EEXIST)
 			err = REFTABLE_LOCK_ERROR;
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 37510c2b2a5..62da3e37823 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -423,6 +423,33 @@ test_expect_success 'ref transaction: fails gracefully when auto compaction fail
 	)
 '
 
+test_expect_success 'ref transaction: timeout acquiring tables.list lock' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit initial &&
+		>.git/reftable/tables.list.lock &&
+		test_must_fail git update-ref refs/heads/branch HEAD 2>err &&
+		test_grep "cannot lock references" err
+	)
+'
+
+test_expect_success 'ref transaction: retry acquiring tables.list lock' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit initial &&
+		LOCK=.git/reftable/tables.list.lock &&
+		>$LOCK &&
+		{
+			( sleep 1 && rm -f $LOCK ) &
+		} &&
+		git -c reftable.lockTimeout=5000 update-ref refs/heads/branch HEAD
+	)
+'
+
 test_expect_success 'pack-refs: compacts tables' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&

-- 
2.46.0.551.gc5ee8f2d1c.dirty
Previous: Patrick SteinhardtNext: karthik nayak
Message 2 of 42 in “reftable: graceful concurrent writes”
  1. 0/3 reftable: graceful concurrent writesPatrick Steinhardt, Sep 17, 2024
  2. 1/3 refs/reftable: introduce "reftable.lockTimeout"Patrick Steinhardt, Sep 17, 2024
  3. karthik nayakSep 17, 2024
  4. Eric SunshineSep 17, 2024
  5. Patrick SteinhardtSep 18, 2024
  6. Patrick SteinhardtSep 18, 2024
  7. 2/3 reftable/stack: allow locking of outdated stacksPatrick Steinhardt, Sep 17, 2024
  8. 3/3 refs/reftable: reload locked stack when preparing transactionPatrick Steinhardt, Sep 17, 2024
  9. karthik nayakSep 17, 2024
  10. Patrick SteinhardtSep 18, 2024
  11. 0/3 reftable: graceful concurrent writesPatrick Steinhardt, Sep 18, 2024
  12. 1/3 refs/reftable: introduce "reftable.lockTimeout"Patrick Steinhardt, Sep 18, 2024
  13. James LiuSep 18, 2024
  14. Patrick SteinhardtSep 18, 2024
  15. 2/3 reftable/stack: allow locking of outdated stacksPatrick Steinhardt, Sep 18, 2024
  16. James LiuSep 18, 2024
  17. Patrick SteinhardtSep 18, 2024
  18. 3/3 refs/reftable: reload locked stack when preparing transactionPatrick Steinhardt, Sep 18, 2024
  19. James LiuSep 18, 2024
  20. 0/3 reftable: graceful concurrent writesPatrick Steinhardt, Sep 18, 2024
  21. 1/3 refs/reftable: introduce "reftable.lockTimeout"Patrick Steinhardt, Sep 18, 2024
  22. Junio C HamanoSep 19, 2024
  23. 2/3 reftable/stack: allow locking of outdated stacksPatrick Steinhardt, Sep 18, 2024
  24. Junio C HamanoSep 20, 2024
  25. Patrick SteinhardtSep 24, 2024
  26. 3/3 refs/reftable: reload locked stack when preparing transactionPatrick Steinhardt, Sep 18, 2024
  27. James LiuSep 18, 2024
  28. Patrick SteinhardtSep 24, 2024
  29. 0/3 reftable: graceful concurrent writesPatrick Steinhardt, Sep 24, 2024
  30. 1/3 refs/reftable: introduce "reftable.lockTimeout"Patrick Steinhardt, Sep 24, 2024
  31. 2/3 reftable/stack: allow locking of outdated stacksPatrick Steinhardt, Sep 24, 2024
  32. 3/3 refs/reftable: reload locked stack when preparing transactionPatrick Steinhardt, Sep 24, 2024
  33. Jeff KingSep 27, 2024
  34. Patrick SteinhardtSep 30, 2024
  35. Josh SteadmonSep 30, 2024
  36. Patrick SteinhardtOct 1, 2024
  37. Jeff KingOct 1, 2024
  38. Junio C HamanoOct 1, 2024
  39. Patrick SteinhardtOct 2, 2024
  40. Junio C HamanoOct 1, 2024
  41. Josh SteadmonOct 1, 2024
  42. Jeff KingOct 1, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.