From patchwork Tue Nov 21 07:04:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462545 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Euh9Tezp"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="M39H+IVM" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 16FF9100 for ; Mon, 20 Nov 2023 23:04:14 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.nyi.internal (Postfix) with ESMTP id E35185C14C5; Tue, 21 Nov 2023 02:04:12 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 21 Nov 2023 02:04:12 -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=1700550252; x=1700636652; bh=w+ BVaHKSDw9Qpi+DFwkruhGex3t4z5YjavfLYPsyIWU=; b=Euh9TezpzM3ZV5B04h PO7ey5Ti4ClWRiF9aS6wQ+EH6i9Ra7Q6AbC0Bsnuc4fNsIwEvUoAJ/v7hSLVzkKX iDuibe6EAthorI4oYMpvbgLlw3QyO5J1X6hmbHYlLc5sq7ILoKZdBXLIbaDU09S3 T9QnhDVsKRA1i5jpKb8NYD0WhmN92X0Rm0KaXfDY2wg6ajuC8rr7D/Xa45YLOIN8 pBKAucad6EOi6+32/zjukxe5fdYtWdW8BVivr0GUmgKB0gSslCYa0vHl9/pYOR9U LkqZ6qkZ2KqrpojSJamhXinuKJ00SjN5MS+O0kZS9YCvqq28/BUog8bNRf8hH/k6 jALA== 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=1700550252; x=1700636652; bh=w+BVaHKSDw9Qp i+DFwkruhGex3t4z5YjavfLYPsyIWU=; b=M39H+IVMZIMa4LuEsECetn2sqTx7S P5t6EPD6BAXzzgrt/3yvtHeUVONw7ufHOQoOxiUt0omUkSLlH0kUneM5yir4fjk5 g3Igcu2ZtNk+c1sccsFKRqrAUmUELXDiBU2hl+wIYqWbdmCG6fL5vSiR4exXpNQU BQjsOlLJOoq4g5BhkL6ZV2NPcECwYXKtYXIruaYVCZVgviGqwgdDEaVyA5eCjLNw bQkb2PhMt2vk9zAvLQ2mUTeUHVJN3Zh8SVpNhaRhY/5sCIlYN//BYIxGD74eeIwx y6DOiYlq0lR+4ubxlFbSoLMwAfbjO812AZFkfTvE1MvADuhEjNoSId6Gw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:11 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 4c49690e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:19 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:10 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 1/8] reftable: wrap EXPECT macros in do/while Message-ID: <89a953135573d028ba7769953f50bf3f43e57d9c.1700549493.git.ps@pks.im> 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: The `EXPECT` macros used by the reftable test framework are all using a single `if` statement with the actual condition. This results in weird syntax when using them in if/else statements like the following: ``` if (foo) EXPECT(foo == 2) else EXPECT(bar == 2) ``` Note that there need not be a trailing semicolon. Furthermore, it is not immediately obvious whether the else now belongs to the `if (foo)` or whether it belongs to the expanded `if (foo == 2)` from the macro. Fix this by wrapping the macros in a do/while loop. Signed-off-by: Patrick Steinhardt --- reftable/test_framework.h | 58 +++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/reftable/test_framework.h b/reftable/test_framework.h index 774cb275bf..ee44f735ae 100644 --- a/reftable/test_framework.h +++ b/reftable/test_framework.h @@ -12,32 +12,38 @@ license that can be found in the LICENSE file or at #include "system.h" #include "reftable-error.h" -#define EXPECT_ERR(c) \ - if (c != 0) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \ - __FILE__, __LINE__, c, reftable_error_str(c)); \ - abort(); \ - } - -#define EXPECT_STREQ(a, b) \ - if (strcmp(a, b)) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \ - __LINE__, #a, a, #b, b); \ - abort(); \ - } - -#define EXPECT(c) \ - if (!(c)) { \ - fflush(stderr); \ - fflush(stdout); \ - fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \ - __LINE__, #c); \ - abort(); \ - } +#define EXPECT_ERR(c) \ + do { \ + if (c != 0) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \ + __FILE__, __LINE__, c, reftable_error_str(c)); \ + abort(); \ + } \ + } while (0) + +#define EXPECT_STREQ(a, b) \ + do { \ + if (strcmp(a, b)) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \ + __LINE__, #a, a, #b, b); \ + abort(); \ + } \ + } while (0) + +#define EXPECT(c) \ + do { \ + if (!(c)) { \ + fflush(stderr); \ + fflush(stdout); \ + fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \ + __LINE__, #c); \ + abort(); \ + } \ + } while (0) #define RUN_TEST(f) \ fprintf(stderr, "running %s\n", #f); \ From patchwork Tue Nov 21 07:04:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462547 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="JoTjhS4a"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="e/0UIb4o" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 95E3BBC for ; Mon, 20 Nov 2023 23:04:17 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 059BE5C1513; Tue, 21 Nov 2023 02:04:17 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 21 Nov 2023 02:04:17 -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=1700550257; x=1700636657; bh=Tm 9E5VGQHgq1lrDzSMzoyqwEolKwlf6hwea70i+QpmM=; b=JoTjhS4aVvJdSShtZC +YgatMS84Deamsk8I24PqV/EpvRUJKJ9XMKR2JXyC3kUB8iQs521MNe89cmF3eCb Fsz2XCk3xNuz9rKoW6Vx2/Q6H8oQ1o7u8AhoQwMsKBt6mO3folk2u8t9tKMbv08y xsSsAkO88Uty0d1Y4QD5l/c8lDVTlJMDrOJJb4RaiS8T1+RHoWHXYGzvAHEXgxvn PWwjU+VX1BuEsxWPo5wT85CFjY+6xUkMx+J6KTxD3eiYzaPlpqf63Zb7KH31iyu8 5gHuOR10+iG2w4LXjAJZy3/+5p/xX0OUUyuCd9wvOYCMbP+KOEb4Vwe6wJBBwF/A VZ8A== 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=1700550257; x=1700636657; bh=Tm9E5VGQHgq1l rDzSMzoyqwEolKwlf6hwea70i+QpmM=; b=e/0UIb4oa6S0DHbkX5mDMu7rXnxtx oacK0FwMTU6PO89BmXsv4yl8uFVKn5WlIrvrfo/kO61guyV+yxGd20t1f6LwRgRr GOmSbESybjbIobEEIESPTcAA9WSVQmsrnxVVPRJPilyU58d564b18Yax+coTjfAq i8uW2C18ZHDqiUMOYbuJeLFIaxMfaFa/8Z1+I7Li4f05mfavONF/NZ8oc7mb4Pv8 KtM+DPgl77IN73N25CoTgluaExWzKpY0yeQ9XeBhdKerk7TK1LCyuJKUF03tmtKq 16Zfp3b4DRYRaSPU0Rwyd2LkgBmbDMY8PlsD2DZabc/B2/1KAlm3EdTwQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:16 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ed4c8ac8 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:24 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:14 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 2/8] reftable: handle interrupted reads 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: There are calls to pread(3P) and read(3P) where we don't properly handle interrupts. Convert them to use `pread_in_full()` and `read_in_full()`, respectively. Signed-off-by: Patrick Steinhardt --- reftable/blocksource.c | 2 +- reftable/stack.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/reftable/blocksource.c b/reftable/blocksource.c index 8331b34e82..a1ea304429 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -109,7 +109,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off, struct file_block_source *b = v; assert(off + size <= b->size); dest->data = reftable_malloc(size); - if (pread(b->fd, dest->data, size, off) != size) + if (pread_in_full(b->fd, dest->data, size, off) != size) return -1; dest->len = size; return size; diff --git a/reftable/stack.c b/reftable/stack.c index ddbdf1b9c8..ed108a929b 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -92,7 +92,7 @@ static int fd_read_lines(int fd, char ***namesp) } buf = reftable_malloc(size + 1); - if (read(fd, buf, size) != size) { + if (read_in_full(fd, buf, size) != size) { err = REFTABLE_IO_ERROR; goto done; } From patchwork Tue Nov 21 07:04:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462548 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="gd3Wf6j9"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="nNQHRLdH" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C360AF4 for ; Mon, 20 Nov 2023 23:04:21 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 39DF25C1575; Tue, 21 Nov 2023 02:04:21 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Tue, 21 Nov 2023 02:04:21 -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=1700550261; x=1700636661; bh=rn INZel+ZAkidiH5ybksGw1A/zgbtkxUHLUMfClGedM=; b=gd3Wf6j9ppocwiudOz vpsBvHkX/2gOWa/cm8ZynPEmD2MNUnyc8ZmTEXfZtlAE+Dfl/9pssm6trB9+82OH NwdeSgZ189trE2Jg4nxf13UbiJ52I+HFZTc6hZdm4hBTNX0HYvFdswbFGxcRclHu /9LGptX0bYAiwkRxUlvuRKlWRWKIXZTgbUei1/avPI51QeCepzzrw1ymostFHq/x xYOyabbzCiYIzPU/WqGOX5tr9NPYwTz2l1v9v04NvElfWOV5FsQ0+j4W/fPDkmcY dj1oXVSI5vLWAIzhfBY8WtxPYv0k9HZSy4bT75h8NqTrmWjkGmbJHdAQHdSCJbVZ 0aJw== 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=1700550261; x=1700636661; bh=rnINZel+ZAkid iH5ybksGw1A/zgbtkxUHLUMfClGedM=; b=nNQHRLdHu9heU3mQB8uu5YSRZkv75 ezqF3+CCAlPIETyYZCTCddkEwUJs5fZC5lfngtIw9u998YFggP0HXWIoavvRoBRr Pl2yaQBfsfJ7zA/na+omhk+iA/TALAn2S5zbNes5ZiFlAoREyM+Nr/SozdGVkMUY y8TN0BfEhThlVME3gaknr+bTm0iVzj0dZDjI6OK6Do5Q22YeuYckujVNWRXZOTUD lEha90s2DJ1QobLm6Mnja8fDtCKJIPnLJ46AOxnQ/3XuvGmSUtvdXvrRmm4kH+Bt jzQrhwT4370NIZU3xHW4S5Tzoj8T4ppa5SnEa+HUYfzyY+Y8Bq+Dv4JTw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:20 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 4f90c186 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:28 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:18 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 3/8] reftable: handle interrupted writes Message-ID: <3c14f67c441251d7467757706715ca6d9a4be7c0.1700549493.git.ps@pks.im> 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: There are calls to write(3P) where we don't properly handle interrupts. Convert them to use `write_in_full()`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 6 +++--- reftable/stack_test.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index ed108a929b..f0cadad490 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -42,7 +42,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st, static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz) { int *fdp = (int *)arg; - return write(*fdp, data, sz); + return write_in_full(*fdp, data, sz); } int reftable_new_stack(struct reftable_stack **dest, const char *dir, @@ -554,7 +554,7 @@ int reftable_addition_commit(struct reftable_addition *add) strbuf_addstr(&table_list, "\n"); } - err = write(add->lock_file_fd, table_list.buf, table_list.len); + err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len); strbuf_release(&table_list); if (err < 0) { err = REFTABLE_IO_ERROR; @@ -1024,7 +1024,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, strbuf_addstr(&ref_list_contents, "\n"); } - err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len); + err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index d0b717510f..0644c8ad2e 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -78,7 +78,7 @@ static void test_read_file(void) int i = 0; EXPECT(fd > 0); - n = write(fd, out, strlen(out)); + n = write_in_full(fd, out, strlen(out)); EXPECT(n == strlen(out)); err = close(fd); EXPECT(err >= 0); From patchwork Tue Nov 21 07:04:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462549 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="ozDE+463"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="FrnIQt3o" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D014610C for ; Mon, 20 Nov 2023 23:04:25 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 4B34F5C1597; Tue, 21 Nov 2023 02:04:25 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 21 Nov 2023 02:04:25 -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=1700550265; x=1700636665; bh=1Y 6wB9dBN5gumrqePPI6bOSR9DQiT3zAXzCTHMYcbpg=; b=ozDE+4633WuGAou1G7 RTSr94FEhlakqjZPzCK5xJS+yPS9/dgXSvkybZOaB3hjeHFbQGKK8mtNFytkTaYT MBIByuJWl1WUguA0e5pKEP8tE5XyutjZ5GoeIpfrk+1QnjsvO394ZzDDB3T/xq0R Zr9tXmLj3xHDF/wIQo88wMpYp+giKR7+MwGxKl9MpjoYW7N4y6u/owSB8xt1Tbl9 7l/g5+8EH/68G9/BsVkScv0KMkRYwfbUUZoTKq9ek2VvJU0V/Lcu0Rni/xzQ/IOw +QmHmaPMra3T49oYpjyhB9NasFI/Bfo9T61HghmaRNT3yNixlbPg6ij9F2GGkKmO AjEQ== 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=1700550265; x=1700636665; bh=1Y6wB9dBN5gum rqePPI6bOSR9DQiT3zAXzCTHMYcbpg=; b=FrnIQt3oKFIHmiuWsfmmV9mTonIr6 vyLqMVzvklm5yXBfg+8LBNW9/sonTbmqRuWNJ7EUF9VH0MJUppjqhE/+YQCXMmwD zwpk4FW9lTYF7ANkcR5oQTyYjL58oM3zHy7yvD71NrvS1fVDgAkYQpbf4/8h7qLK LIP/LnUAwIQMNIfWzOij1Q/pr/zWVxNAZrrxk2QYB7PXgZIo7tfqnEGm8FD8HbH4 TBfZ3Tb9yGvzyAavQ/1PWt6IPYqrah2IRpmFopqtX54dEu7ENug5Et3Wsfe2Olq/ BdVY877rXcRH+gadH4HseQZOTMMtuASzre2hO/auRbkjN9gwnuKLX8+bQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:24 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id fa1e6ac6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:32 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:22 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 4/8] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction 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: While we have several tests that check whether we correctly perform auto-compaction when manually calling `reftable_stack_auto_compact()`, we don't have any tests that verify whether `reftable_stack_add()` does call it automatically. Add one. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 47 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 0644c8ad2e..c979d177c2 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -850,6 +850,52 @@ static void test_reftable_stack_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_add_performs_auto_compaction(void) +{ + struct reftable_write_options cfg = { 0 }; + struct reftable_stack *st = NULL; + char *dir = get_tmp_dir(__LINE__); + int err, i, n = 20; + + 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]; + + /* + * 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; + + snprintf(name, sizeof(name), "branch%04d", i); + ref.refname = name; + + err = reftable_stack_add(st, &write_test_ref, &ref); + EXPECT_ERR(err); + + /* + * 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_compaction_concurrent(void) { struct reftable_write_options cfg = { 0 }; @@ -960,6 +1006,7 @@ 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_add_performs_auto_compaction); RUN_TEST(test_reftable_stack_compaction_concurrent); RUN_TEST(test_reftable_stack_compaction_concurrent_clean); RUN_TEST(test_reftable_stack_hash_id); From patchwork Tue Nov 21 07:04:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462550 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="FaVdEUf6"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="E3mVsa9u" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EA50610C for ; Mon, 20 Nov 2023 23:04:29 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.nyi.internal (Postfix) with ESMTP id 5D9735C1621; Tue, 21 Nov 2023 02:04:29 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 21 Nov 2023 02:04:29 -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=1700550269; x=1700636669; bh=CI KlOe25goD/oDdsM72nIPnSTgtgH2O4/cTCljUer/8=; b=FaVdEUf60a7GbIZTw/ 5S9USRsnDyivo/TDImNLkmA2BAO0e07h62bhAnHCxXOXgDkh+5DbqK62ZumenFV7 ApMmvs0P7OR9mCf8/RIilpmWzh65PJeAOcjwXtzfDM0dRDAHfjnoCs71RQco8JaM Uns/vBhtliBkCo/+X/Sgn5L/DXaGPT8Xtd7nu0opn6bbCsPmqCx2g4TGzQrvcJGy 2Buw1iiEOEdLX+++/Z2iCIeezIOcd9nFXv3ccaASqCUc3clijPj9x70hppoM/JJV 4trQto33BqW8Mnzh6ENFTM9rV4Qzx34iu/LxmgYGf79GrjIKsE8rXpwhm6lotWFB 7xJg== 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=1700550269; x=1700636669; bh=CIKlOe25goD/o DdsM72nIPnSTgtgH2O4/cTCljUer/8=; b=E3mVsa9ucduZv/r+ptEcF9z1vUUkp U8qan1LJLNVQ4WH47F/E96ksynfP12ESUHzuZR1TPEasNmRraLvZ3Mysi+C3H2xM +uuX2dEbzpdu6g4xIu6gVXd3YOxK5HjrUL8joiQRPQEppK5v8cO44K0IVBUZrrYz 9eRR45bEFAf221Bo4tysT+fY0eDLdFV2qsZLnsbaj6iCauZ6TTS9859sLNkcRovC yPUgOPw66uw7c+PmSP7bO4MAMo3EOlmar8nsUJCgDnCmxllxnQl07iv3QhBCNnYJ VQS4NvpqOyU613eJC4Z0R1/hSJE4HdNAEiZzlyVQxv1VTlSQJLrkLbjwQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:28 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 12929a02 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:36 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:26 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 5/8] reftable/stack: perform auto-compaction with transactional interface Message-ID: <25522b042cdc5986972cc7b62e6b88be0569d3cb.1700549493.git.ps@pks.im> 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. It can thus happen quite fast that the stack grows very long, which results in 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 c979d177c2..4c2f794c49 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 }; @@ -1014,6 +1069,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); From patchwork Tue Nov 21 07:04:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462551 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="XyodcoBR"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="q3H0rT4R" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00C7790 for ; Mon, 20 Nov 2023 23:04:33 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 6EB8D5C1662; Tue, 21 Nov 2023 02:04:33 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 21 Nov 2023 02:04:33 -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=1700550273; x=1700636673; bh=KU N1u+yN84USZoxraciClmYF5T+bNUyMhMKQZ+Daqh0=; b=XyodcoBRmPMQ5vWhbP TDK0EfmJDHyTwcrwi3oSYjHFL8Fo12dHdtZrjuew2zTyjBnZFqOkhK60DqlD4xbQ iIk6j1E0Z0oHQae9YrhQddOzQORECHaUym6T4BBn50EQkkP4bFkp19sGNqdD4zXN EqupxcX1w1jf0655CuEEC1cX2/9wms9Qif/j08paVk8zKDMPsxI3xNhUaOnyFMdJ FsVR1eHg6D4ayFBu4Td8l7oULsuAQ9ct5RJYgJhWkM1urDnDG6Usm1qjhcbYcqlX olL+1dBgRp/eDFVryL2nJkOJjOi9aiyfU5IYqyl1eZ/lDhGQtlDAQ+Hg9o+z6uq3 rmPA== 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=1700550273; x=1700636673; bh=KUN1u+yN84USZ oxraciClmYF5T+bNUyMhMKQZ+Daqh0=; b=q3H0rT4RRnlWX01oi7fxyA6YgqS2w Y7R8xfFbCkcl/gzbGnwbmVAbNtviLxqNJMjy+/IY5qjMXIOXIafoBxaFWkH6vVo+ aFqS+KWu4UsLeYuxFQ3/TQtDpjKOoup44TQpK9Ho7ic4IjUFpMn+HVi88fW34kH3 TTUw1QNzXjxerKpdMe2LiZwSYWZRHXdjjy4lL6adY3ZbXDr0Dh36JqFJid384EVH +DjvJDX39wMuR5f73HU5pBbRP1q8tjtvtpoQHFXsbqeObdD4LOZbGGApl0a4CAB6 6fK3PIUF1ZdhOOT8p2BUEfYSEPMlGPS0zWrW/NbVeht1fTN3Vj4vtKGDQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:32 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 9f72fbc2 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:40 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:30 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 6/8] reftable/stack: reuse buffers when reloading stack Message-ID: <54e8fd15d8ab3266b1159045b1029b2a38f447a7.1700549493.git.ps@pks.im> 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: In `reftable_stack_reload_once()` we iterate over all the tables added to the stack in order to figure out whether any of the tables needs to be reloaded. We use a set of buffers in this context to compute the paths of these tables, but discard those buffers on every iteration. This is quite wasteful given that we do not need to transfer ownership of the allocated buffer outside of the loop. Refactor the code to instead reuse the buffers to reduce the number of allocations we need to do. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index f5d18a842a..2dd2373360 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -204,6 +204,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, reftable_calloc(sizeof(struct reftable_table) * names_len); int new_readers_len = 0; struct reftable_merged_table *new_merged = NULL; + struct strbuf table_path = STRBUF_INIT; int i; while (*names) { @@ -223,13 +224,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, if (!rd) { struct reftable_block_source src = { NULL }; - struct strbuf table_path = STRBUF_INIT; stack_filename(&table_path, st, name); err = reftable_block_source_from_file(&src, table_path.buf); - strbuf_release(&table_path); - if (err < 0) goto done; @@ -267,16 +265,13 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, for (i = 0; i < cur_len; i++) { if (cur[i]) { const char *name = reader_name(cur[i]); - struct strbuf filename = STRBUF_INIT; - stack_filename(&filename, st, name); + stack_filename(&table_path, st, name); reader_close(cur[i]); reftable_reader_free(cur[i]); /* On Windows, can only unlink after closing. */ - unlink(filename.buf); - - strbuf_release(&filename); + unlink(table_path.buf); } } @@ -288,6 +283,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names, reftable_free(new_readers); reftable_free(new_tables); reftable_free(cur); + strbuf_release(&table_path); return err; } From patchwork Tue Nov 21 07:04:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462552 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="mXySXk++"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="noXh2FS3" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AE2A90 for ; Mon, 20 Nov 2023 23:04:38 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 7A6B35C16B0; Tue, 21 Nov 2023 02:04:37 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Tue, 21 Nov 2023 02:04:37 -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=1700550277; x=1700636677; bh=OY rVHvJw9rPBjDo146T+1ZsiZSRz31wEIj7NrpAnM5Y=; b=mXySXk++4W0hMEoKDA bVXazZ8mL2nJ4DLfDrxGGOQvEjp0fx5XXkiNEIEQOKjxm06zwTorrusXD5G3xQIF 5GRO255Llrcw8xCeajGChL0hqCMXqjiwvlplSLTvZFHneFnd5rtUdi2iKLZziAqZ 9jsQAt8sxJEdC0ZIEGhK64acyYdfFwcVsNbPxtOR3SZJH/O7HENIDYQM8LKukp+e I5NU5AxYX5YIJKIvgcvZiAgyxhFF5mHOaygw7nULX+i2DhQNCDXRsGvDhKxyRfFO L9n/Rbt8bp9Wb5K13xxeQQk3kz700lu/EQ5eR1G2x9EFT1bNRTK7XyDX7bsdjhHw 5xcA== 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=1700550277; x=1700636677; bh=OYrVHvJw9rPBj Do146T+1ZsiZSRz31wEIj7NrpAnM5Y=; b=noXh2FS3mCbYtIqtoTq0od4nOUy9e yknb5jZICGYQketZbWflXiGNBpkl9fuKs1uu913OgNYOslk6Y8sAvM7j3Y7baj/A 6eKR4DZzLttLJCCtmnJLf/U8roi67fAqm3hhw2ifFLldkSQVoPPzMU8yRNgDmwA5 f/G8ycEq1nJUX4Ut0i5fkpnNJpLuOSujUphs2LYBDORYvyEwcUSRsjFWLU7R/jFw LIrzQ8JTir2YNy0A1ySaCsSZou9X8mXWJSu4m+ofKPvyByhpC70CRJkYn9HZDzTl 4jWuDfWiKRCkGxyxtLOr8TjTu8YG8ORENd9fvv4JsBX1F9XXor/zj3ZMQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:36 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6737a536 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:44 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:35 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 7/8] reftable/merged: reuse buffer to compute record keys Message-ID: <23c060d1e21573581ca6c5db50ca756b61078e3e.1700549493.git.ps@pks.im> 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: When iterating over entries in the merged iterator's queue, we compute the key of each of the entries and write it into a buffer. We do not reuse the buffer though and thus re-allocate it on every iteration, which is wasteful given that we never transfer ownership of the allocated bytes outside of the loop. Refactor the code to reuse the buffer. This also fixes a potential memory leak when `merged_iter_advance_subiter()` returns an error. Signed-off-by: Patrick Steinhardt --- reftable/merged.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index 5ded470c08..cd03f6da13 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -85,7 +85,7 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx) static int merged_iter_next_entry(struct merged_iter *mi, struct reftable_record *rec) { - struct strbuf entry_key = STRBUF_INIT; + struct strbuf entry_key = STRBUF_INIT, k = STRBUF_INIT; struct pq_entry entry = { 0 }; int err = 0; @@ -108,30 +108,28 @@ static int merged_iter_next_entry(struct merged_iter *mi, reftable_record_key(&entry.rec, &entry_key); while (!merged_iter_pqueue_is_empty(mi->pq)) { struct pq_entry top = merged_iter_pqueue_top(mi->pq); - struct strbuf k = STRBUF_INIT; - int err = 0, cmp = 0; + int cmp = 0; reftable_record_key(&top.rec, &k); cmp = strbuf_cmp(&k, &entry_key); - strbuf_release(&k); - - if (cmp > 0) { + if (cmp > 0) break; - } merged_iter_pqueue_remove(&mi->pq); err = merged_iter_advance_subiter(mi, top.index); - if (err < 0) { - return err; - } + if (err < 0) + goto done; reftable_record_release(&top.rec); } reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id)); + +done: reftable_record_release(&entry.rec); strbuf_release(&entry_key); - return 0; + strbuf_release(&k); + return err; } static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec) From patchwork Tue Nov 21 07:04:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13462553 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="qDruGn10"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="gB7PJR7T" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0B0B990 for ; Mon, 20 Nov 2023 23:04:42 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 789015C16F6; Tue, 21 Nov 2023 02:04:41 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Tue, 21 Nov 2023 02:04:41 -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=1700550281; x=1700636681; bh=q4 lEyws93ZJX6pimXEwvcwRWXb7+dfvNrihseQkQfm4=; b=qDruGn10inPacAyDji MUtm8lti/rAm4JSLwrnHVHc5Zq6yY4zbXz6uuFQuT7RowSeAzIGG5enLZZK8rv5U yKWBgyuTH1hTCxVWknJbFbRNVKbIP/EDOlEgQWmD1wXd4DOZvGcd6+d/rP412Ski Fvz+fsV/HWG5BgVDQqz18atNBlm7g2VLY4xu9Axf+/w+TxCH+RZYcUXB3F3PnkxX QCzJYFN5xGQUCzxnbQ7Pw6bwiaXAES7PmoZbHt8cIB/XPct0h2nG7w7GBZf3FXEK aNQ+tvJNBP0zbxNB8PQCJMS7dhcv+oSQ0ElbBrH7U9L7RKcjmc+Bef/bdnQCRckQ /wxg== 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=1700550281; x=1700636681; bh=q4lEyws93ZJX6 pimXEwvcwRWXb7+dfvNrihseQkQfm4=; b=gB7PJR7TIdnG3zEZoW5gGDJXuIheN NkWk1Oy76fBMn433Krif9WhmKJuZFBbYDSdGR6OFfljade0rzYEoO/YdzPTOxlnM lbvCOe92oBiqifYk2B6ID9+fUaVhLt2sLzPI9DxtgvLhKlb9GA5XbtDXUcWslPjC YgP6ZyE4sw7TurPKaGfqtrr58KQgBarWBIy0JRO1BODMxJmo2ffwaU1bZFzTzIJf rANm6JyW7+4E8cbR7AVjw04rRoQcLv+rSsiwCnQwNF195MS9+/OJ9hC/VNIplOj8 0Qxn18WljugH7QQJ2T5okz8IXy7foPjPjT7kBn+/PZaLF3DSxCob7OaiA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudegkedguddtfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleff teenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpsh esphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Nov 2023 02:04:40 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6524aec5 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 21 Nov 2023 07:03:48 +0000 (UTC) Date: Tue, 21 Nov 2023 08:04:39 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH 8/8] reftable/stack: fix stale lock when dying Message-ID: <065c8803ac18633313af264f70c83717c4f6e10c.1700549493.git.ps@pks.im> 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: When starting a transaction via `reftable_stack_init_addition()`, we create a lockfile for the reftable stack itself which we'll write the new list of tables to. But if we terminate abnormally e.g. via a call to `die()`, then we do not remove the lockfile. Subsequent executions of Git which try to modify references will thus fail with an out-of-date error. Fix this bug by registering the lock as a `struct tempfile`, which ensures automatic cleanup for us. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 47 +++++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 2dd2373360..2f1494aef2 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -17,6 +17,8 @@ license that can be found in the LICENSE file or at #include "reftable-merged.h" #include "writer.h" +#include "tempfile.h" + static int stack_try_add(struct reftable_stack *st, int (*write_table)(struct reftable_writer *wr, void *arg), @@ -440,8 +442,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max) } struct reftable_addition { - int lock_file_fd; - struct strbuf lock_file_name; + struct tempfile *lock_file; struct reftable_stack *stack; char **new_tables; @@ -449,24 +450,19 @@ struct reftable_addition { uint64_t next_update_index; }; -#define REFTABLE_ADDITION_INIT \ - { \ - .lock_file_name = STRBUF_INIT \ - } +#define REFTABLE_ADDITION_INIT {0} static int reftable_stack_init_addition(struct reftable_addition *add, struct reftable_stack *st) { + struct strbuf lock_file_name = STRBUF_INIT; int err = 0; add->stack = st; - strbuf_reset(&add->lock_file_name); - strbuf_addstr(&add->lock_file_name, st->list_file); - strbuf_addstr(&add->lock_file_name, ".lock"); + strbuf_addf(&lock_file_name, "%s.lock", st->list_file); - add->lock_file_fd = open(add->lock_file_name.buf, - O_EXCL | O_CREAT | O_WRONLY, 0666); - if (add->lock_file_fd < 0) { + add->lock_file = create_tempfile(lock_file_name.buf); + if (!add->lock_file) { if (errno == EEXIST) { err = REFTABLE_LOCK_ERROR; } else { @@ -475,7 +471,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; } if (st->config.default_permissions) { - if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) { + if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; } @@ -495,6 +491,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, if (err) { reftable_addition_close(add); } + strbuf_release(&lock_file_name); return err; } @@ -512,15 +509,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables = NULL; add->new_tables_len = 0; - if (add->lock_file_fd > 0) { - close(add->lock_file_fd); - add->lock_file_fd = 0; - } - if (add->lock_file_name.len > 0) { - unlink(add->lock_file_name.buf); - strbuf_release(&add->lock_file_name); - } - + delete_tempfile(&add->lock_file); strbuf_release(&nm); } @@ -536,8 +525,10 @@ void reftable_addition_destroy(struct reftable_addition *add) int reftable_addition_commit(struct reftable_addition *add) { struct strbuf table_list = STRBUF_INIT; + int lock_file_fd = get_tempfile_fd(add->lock_file); int i = 0; int err = 0; + if (add->new_tables_len == 0) goto done; @@ -550,28 +541,20 @@ int reftable_addition_commit(struct reftable_addition *add) strbuf_addstr(&table_list, "\n"); } - err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len); + err = write_in_full(lock_file_fd, table_list.buf, table_list.len); strbuf_release(&table_list); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } - err = close(add->lock_file_fd); - add->lock_file_fd = 0; - if (err < 0) { - err = REFTABLE_IO_ERROR; - goto done; - } - - err = rename(add->lock_file_name.buf, add->stack->list_file); + err = rename_tempfile(&add->lock_file, add->stack->list_file); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; } /* success, no more state to clean up. */ - strbuf_release(&add->lock_file_name); for (i = 0; i < add->new_tables_len; i++) { reftable_free(add->new_tables[i]); }