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

[PATCH 2/8] reftable/stack: test compaction with already-locked tables

From
Patrick Steinhardt <ps@pks.im>
Date
Jul 31, 2024, 14:14 UTC
Message-ID
<123fb9d80eecbd3690280991e0415cbb718b7202.1722435214.git.ps@pks.im>
In-Reply-To
<cover.1722435214.git.ps@pks.im>

We're lacking test coverage for compacting tables when some of the tables that we are about to compact are locked. Add two tests that exercise this, one for auto-compaction and one for full compaction.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack_test.c | 103 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index e3c11e6a6e..04526c6604 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -863,6 +863,58 @@ static void test_reftable_stack_auto_compaction(void)
 	clear_dir(dir);
 }
 
+static void test_reftable_stack_auto_compaction_with_locked_tables(void)
+{
+	struct reftable_write_options opts = {
+		.disable_auto_compact = 1,
+	};
+	struct reftable_stack *st = NULL;
+	struct strbuf buf = STRBUF_INIT;
+	char *dir = get_tmp_dir(__LINE__);
+	int err;
+
+	err = reftable_new_stack(&st, dir, &opts);
+	EXPECT_ERR(err);
+
+	for (size_t i = 0; i < 5; i++) {
+		struct reftable_ref_record ref = {
+			.update_index = reftable_stack_next_update_index(st),
+			.value_type = REFTABLE_REF_VAL1,
+			.value.val1 = { i },
+		};
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "refs/heads/branch-%04" PRIuMAX, (uintmax_t) i);
+		ref.refname = buf.buf;
+
+		err = reftable_stack_add(st, &write_test_ref, &ref);
+		EXPECT_ERR(err);
+	}
+	EXPECT(st->merged->stack_len == 5);
+
+	/*
+	 * Given that all tables we have written should be roughly the same
+	 * size, we expect that auto-compaction will want to compact all of the
+	 * tables. Locking any of the tables will keep it from doing so.
+	 */
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[2]->name);
+	write_file_buf(buf.buf, "", 0);
+
+	/*
+	 * Ideally, we'd handle the situation where any of the tables is locked
+	 * gracefully. We don't (yet) do this though and thus fail.
+	 */
+	err = reftable_stack_auto_compact(st);
+	EXPECT(err == REFTABLE_LOCK_ERROR);
+	EXPECT(st->stats.failures == 1);
+	EXPECT(st->merged->stack_len == 5);
+
+	reftable_stack_destroy(st);
+	strbuf_release(&buf);
+	clear_dir(dir);
+}
+
 static void test_reftable_stack_add_performs_auto_compaction(void)
 {
 	struct reftable_write_options opts = { 0 };
@@ -911,6 +963,55 @@ static void test_reftable_stack_add_performs_auto_compaction(void)
 	clear_dir(dir);
 }
 
+static void test_reftable_stack_compaction_with_locked_tables(void)
+{
+	struct reftable_write_options opts = {
+		.disable_auto_compact = 1,
+	};
+	struct reftable_stack *st = NULL;
+	struct strbuf buf = STRBUF_INIT;
+	char *dir = get_tmp_dir(__LINE__);
+	int err;
+
+	err = reftable_new_stack(&st, dir, &opts);
+	EXPECT_ERR(err);
+
+	for (size_t i = 0; i < 3; i++) {
+		struct reftable_ref_record ref = {
+			.update_index = reftable_stack_next_update_index(st),
+			.value_type = REFTABLE_REF_VAL1,
+			.value.val1 = { i },
+		};
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "refs/heads/branch-%04" PRIuMAX, (uintmax_t) i);
+		ref.refname = buf.buf;
+
+		err = reftable_stack_add(st, &write_test_ref, &ref);
+		EXPECT_ERR(err);
+	}
+	EXPECT(st->merged->stack_len == 3);
+
+	/* Lock one of the tables that we're about to compact. */
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[1]->name);
+	write_file_buf(buf.buf, "", 0);
+
+	/*
+	 * Compaction is expected to fail given that we were not able to
+	 * compact all tables.
+	 */
+	err = reftable_stack_compact_all(st, NULL);
+	EXPECT(err == REFTABLE_LOCK_ERROR);
+	/* TODO: this is wrong, we should get notified about the failure. */
+	EXPECT(st->stats.failures == 0);
+	EXPECT(st->merged->stack_len == 3);
+
+	reftable_stack_destroy(st);
+	strbuf_release(&buf);
+	clear_dir(dir);
+}
+
 static void test_reftable_stack_compaction_concurrent(void)
 {
 	struct reftable_write_options opts = { 0 };
@@ -1016,9 +1117,11 @@ int stack_test_main(int argc, const char *argv[])
 	RUN_TEST(test_reftable_stack_add);
 	RUN_TEST(test_reftable_stack_add_one);
 	RUN_TEST(test_reftable_stack_auto_compaction);
+	RUN_TEST(test_reftable_stack_auto_compaction_with_locked_tables);
 	RUN_TEST(test_reftable_stack_add_performs_auto_compaction);
 	RUN_TEST(test_reftable_stack_compaction_concurrent);
 	RUN_TEST(test_reftable_stack_compaction_concurrent_clean);
+	RUN_TEST(test_reftable_stack_compaction_with_locked_tables);
 	RUN_TEST(test_reftable_stack_hash_id);
 	RUN_TEST(test_reftable_stack_lock_failure);
 	RUN_TEST(test_reftable_stack_log_normalize);
-- 
2.46.0.dirty
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 50 in “reftable: improvements and fixes for compaction”
  1. 0/8 reftable: improvements and fixes for compactionPatrick Steinhardt, Jul 31, 2024
  2. 1/8 reftable/stack: refactor function to gather table sizesPatrick Steinhardt, Jul 31, 2024
  3. Junio C HamanoAug 2, 2024
  4. 2/8 reftable/stack: test compaction with already-locked tablesPatrick Steinhardt, Jul 31, 2024
  5. Junio C HamanoAug 2, 2024
  6. Patrick SteinhardtAug 5, 2024
  7. 3/8 reftable/stack: update stats on failed full compactionPatrick Steinhardt, Jul 31, 2024
  8. 4/8 reftable/stack: simplify tracking of table locksPatrick Steinhardt, Jul 31, 2024
  9. Justin ToblerJul 31, 2024
  10. Junio C HamanoAug 2, 2024
  11. Patrick SteinhardtAug 5, 2024
  12. 5/8 reftable/stack: do not die when fsyncing lock file filesPatrick Steinhardt, Jul 31, 2024
  13. 6/8 reftable/stack: use lock_file when adding table to "tables.list"Patrick Steinhardt, Jul 31, 2024
  14. Justin ToblerJul 31, 2024
  15. Patrick SteinhardtAug 1, 2024
  16. 7/8 reftable/stack: fix corruption on concurrent compactionPatrick Steinhardt, Jul 31, 2024
  17. Justin ToblerAug 1, 2024
  18. Patrick SteinhardtAug 1, 2024
  19. 8/8 reftable/stack: handle locked tables during auto-compactionPatrick Steinhardt, Jul 31, 2024
  20. Junio C HamanoAug 2, 2024
  21. Patrick SteinhardtAug 5, 2024
  22. 0/9 reftable: improvements and fixes for compactionPatrick Steinhardt, Aug 5, 2024
  23. 1/9 reftable/stack: refactor function to gather table sizesPatrick Steinhardt, Aug 5, 2024
  24. 2/9 reftable/stack: extract function to setup stack with N tablesPatrick Steinhardt, Aug 5, 2024
  25. 3/9 reftable/stack: test compaction with already-locked tablesPatrick Steinhardt, Aug 5, 2024
  26. Karthik NayakAug 8, 2024
  27. 4/9 reftable/stack: update stats on failed full compactionPatrick Steinhardt, Aug 5, 2024
  28. 5/9 reftable/stack: simplify tracking of table locksPatrick Steinhardt, Aug 5, 2024
  29. 6/9 reftable/stack: do not die when fsyncing lock file filesPatrick Steinhardt, Aug 5, 2024
  30. 7/9 reftable/stack: use lock_file when adding table to "tables.list"Patrick Steinhardt, Aug 5, 2024
  31. 8/9 reftable/stack: fix corruption on concurrent compactionPatrick Steinhardt, Aug 5, 2024
  32. Karthik NayakAug 8, 2024
  33. Patrick SteinhardtAug 8, 2024
  34. 9/9 reftable/stack: handle locked tables during auto-compactionPatrick Steinhardt, Aug 5, 2024
  35. Justin ToblerAug 6, 2024
  36. Patrick SteinhardtAug 7, 2024
  37. Justin ToblerAug 7, 2024
  38. Karthik NayakAug 8, 2024
  39. Karthik NayakAug 8, 2024
  40. 0/9 reftable: improvements and fixes for compactionPatrick Steinhardt, Aug 8, 2024
  41. 1/9 reftable/stack: refactor function to gather table sizesPatrick Steinhardt, Aug 8, 2024
  42. 2/9 reftable/stack: extract function to setup stack with N tablesPatrick Steinhardt, Aug 8, 2024
  43. 3/9 reftable/stack: test compaction with already-locked tablesPatrick Steinhardt, Aug 8, 2024
  44. 4/9 reftable/stack: update stats on failed full compactionPatrick Steinhardt, Aug 8, 2024
  45. 5/9 reftable/stack: simplify tracking of table locksPatrick Steinhardt, Aug 8, 2024
  46. 6/9 reftable/stack: do not die when fsyncing lock file filesPatrick Steinhardt, Aug 8, 2024
  47. 7/9 reftable/stack: use lock_file when adding table to "tables.list"Patrick Steinhardt, Aug 8, 2024
  48. 8/9 reftable/stack: fix corruption on concurrent compactionPatrick Steinhardt, Aug 8, 2024
  49. 9/9 reftable/stack: handle locked tables during auto-compactionPatrick Steinhardt, Aug 8, 2024
  50. Karthik NayakAug 8, 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.