From patchwork Mon Dec 11 09:07:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13486862 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="TgubDAcw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="aYDw7/5c" Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4BF66D9 for ; Mon, 11 Dec 2023 01:07:51 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 66022320268D; Mon, 11 Dec 2023 04:07:50 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Mon, 11 Dec 2023 04:07:50 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1702285669; x=1702372069; bh=e8 uVX+p/aIJqUt9uBvRrCYzrQgoLhUP0CGh3m2N1vDQ=; b=TgubDAcwRGKcvAXzOE /FbE7ppsShUi8prU0B4Ji6iBYnRwEHvna7t92Pk/bnYA1FggP0Ju9ZkQ26MR2h3M 0tpDT+hhQX82JJNe6L2qha7fKv+GDERgDYUipAzVBapfVEO6kLUzNlqXktyzRpkK ELO09Dyt38eHuDbzp0+mTmE8MKhfc3Bp6zKA7W6kuyRnaUQzmpNiDNq7pjqyDU/K joZfpezRiQZxmiA55c9vaoPEIJx4/qieVhQFW9gYE0sLXywX+GeK11n9RUiBgdLz iluBWg3V87qYls9O2NEaeoDHqKRUvuaU5kP0WMm6buxEywN3SCBktOJVunv+ZdXS xBcw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1702285669; x=1702372069; bh=e8uVX+p/aIJqU t9uBvRrCYzrQgoLhUP0CGh3m2N1vDQ=; b=aYDw7/5c84889yEA7M+Eu+EY7nUZM jFRlqaEJ8Z6l3D5px4RlUIBmm6Y71UFeKvA/0khKlOeManUdrp4+Xee9Y9TzBAlL mRC0hBbXK7GadiqAV99CCm3uc635XglZELqH5pF2MDnPQnTnFQiqHbfMidTFLLql SIyEg5xnfBa7XiVp4mC09hojMjlcrHoTfkuL2G49wRnebB71xT963QtFzNHCyD2H VaIr1f+bR3xPr9NF0yZ9YlR89At71/oJ58R2N5t49NbQyr0hVdW41b1JgaovAeS9 MS5z9SVK+kLw4zIKMNj8Zo0UpekQiwyMsTV6/8l9TRsZAx8OIImhTDiuA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeluddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Dec 2023 04:07:48 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1d1e06ba (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Dec 2023 09:06:12 +0000 (UTC) Date: Mon, 11 Dec 2023 10:07:46 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder , Taylor Blau , Eric Sunshine Subject: [PATCH v3 05/11] reftable/stack: perform auto-compaction with transactional interface Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Whenever updating references or reflog entries in the reftable stack, we need to add a new table to the stack, thus growing the stack's length by one. The stack can grow to become quite long rather quickly, leading to performance issues when trying to read records. But besides performance issues, this can also lead to exhaustion of file descriptors very rapidly as every single table requires a separate descriptor when opening the stack. While git-pack-refs(1) fixes this issue for us by merging the tables, it runs too irregularly to keep the length of the stack within reasonable limits. This is why the reftable stack has an auto-compaction mechanism: `reftable_stack_add()` will call `reftable_stack_auto_compact()` after its added the new table, which will auto-compact the stack as required. But while this logic works alright for `reftable_stack_add()`, we do not do the same in `reftable_addition_commit()`, which is the transactional equivalent to the former function that allows us to write multiple updates to the stack atomically. Consequentially, we will easily run into file descriptor exhaustion in code paths that use many separate transactions like e.g. non-atomic fetches. Fix this issue by calling `reftable_stack_auto_compact()`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 6 +++++ reftable/stack_test.c | 56 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/reftable/stack.c b/reftable/stack.c index f0cadad490..f5d18a842a 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -584,6 +584,12 @@ int reftable_addition_commit(struct reftable_addition *add) add->new_tables_len = 0; err = reftable_stack_reload(add->stack); + if (err) + goto done; + + if (!add->stack->disable_auto_compact) + err = reftable_stack_auto_compact(add->stack); + done: reftable_addition_close(add); return err; diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 52b4dc3b14..14a3fc11ee 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -289,6 +289,61 @@ static void test_reftable_stack_transaction_api(void) clear_dir(dir); } +static void test_reftable_stack_transaction_api_performs_auto_compaction(void) +{ + char *dir = get_tmp_dir(__LINE__); + struct reftable_write_options cfg = {0}; + struct reftable_addition *add = NULL; + struct reftable_stack *st = NULL; + int i, n = 20, err; + + err = reftable_new_stack(&st, dir, cfg); + EXPECT_ERR(err); + + for (i = 0; i <= n; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_SYMREF, + .value.symref = "master", + }; + char name[100]; + + snprintf(name, sizeof(name), "branch%04d", i); + ref.refname = name; + + /* + * Disable auto-compaction for all but the last runs. Like this + * we can ensure that we indeed honor this setting and have + * better control over when exactly auto compaction runs. + */ + st->disable_auto_compact = i != n; + + err = reftable_stack_new_addition(&add, st); + EXPECT_ERR(err); + + err = reftable_addition_add(add, &write_test_ref, &ref); + EXPECT_ERR(err); + + err = reftable_addition_commit(add); + EXPECT_ERR(err); + + reftable_addition_destroy(add); + + /* + * The stack length should grow continuously for all runs where + * auto compaction is disabled. When enabled, we should merge + * all tables in the stack. + */ + if (i != n) + EXPECT(st->merged->stack_len == i + 1); + else + EXPECT(st->merged->stack_len == 1); + } + + reftable_stack_destroy(st); + clear_dir(dir); +} + static void test_reftable_stack_validate_refname(void) { struct reftable_write_options cfg = { 0 }; @@ -1016,6 +1071,7 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_log_normalize); RUN_TEST(test_reftable_stack_tombstone); RUN_TEST(test_reftable_stack_transaction_api); + RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction); RUN_TEST(test_reftable_stack_update_index_check); RUN_TEST(test_reftable_stack_uptodate); RUN_TEST(test_reftable_stack_validate_refname);