From patchwork Fri Dec 8 14:52:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485474 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="J8x4m//Z"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="kWAPfDp6" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4C64F49F9 for ; Fri, 8 Dec 2023 06:53:01 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id DD9313200A1E; Fri, 8 Dec 2023 09:53:00 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 08 Dec 2023 09:53:01 -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=1702047180; x=1702133580; bh=Zz 0iv8TEZagcxXQ9PtKJFySXBoGfoSd0GOGycMwMR04=; b=J8x4m//ZZ4WeXD9Lyx Yxo13WxJs8K5kGiwzHlleatPL3DxlySVxeOxF2OmCaUQ3pZBePylde5lsAUnjTEC Suw/9F0C57w1e7l9VRUWzYl/LpXiKDIOjV94/Gy/06gGB/B0i1Pfv7PlihT7ExMd uPIQowknges/wzsSNwGRme406Bu0mJqNbKAIxgzjvFQkVS7Y4A2O7A1bR7uXxAR8 2G3VpZsoBTWveHiu8nffZCyWC/8HrsgWnEqK5ysvdSv4MlBpzXpNaGu1k/eYovI1 BqfD+3YRrfGh3yP+l3+sE375fnTWzoVe+OXjQcT7IKCz11EODukgKuIm1+rPoVZ3 SXXw== 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=1702047180; x=1702133580; bh=Zz0iv8TEZagcx XQ9PtKJFySXBoGfoSd0GOGycMwMR04=; b=kWAPfDp6qL9sbRwP0ZHk+CcEIr+fh 1tT3NKGMEYfIsbWpNjfPCn8M6y8VVvflxVg8UvK0xHX6NjQcsW3jCEUhKIJKCrz4 besGhoffoXeSYwn3PNuBHcvxA5fUCab0zm6kK+AEVhlhlqjSfv5zmbNBCmCB/s6B T+rRueI2sgAEKRnF8+zQGZxzT5J5lMxhQrbExRk4QN9hWiDfxMa1BWcNgyiG98Y3 CyJC0jb41mwdLJSr2Af377RcflwCnxDaN2HIEBmcwgYUP8NmdpVgMPQJqwsSU+cL 4prpuxMG6wbak8KTf8GTq6EIcQNXVNNr5cdTBPK/vYTTscSRctJ72x3KQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:52:59 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 0d60eb7e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:28 +0000 (UTC) Date: Fri, 8 Dec 2023 15:52:57 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 01/11] reftable: wrap EXPECT macros in do/while Message-ID: <0ebbb02d32e1f1f483c21157fe076c0890665f69.1702047081.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 Fri Dec 8 14:53:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485477 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="YcUsJNC7"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hr4BhDW8" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3DA704C2E for ; Fri, 8 Dec 2023 06:53:06 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 1A5DC320077A; Fri, 8 Dec 2023 09:53:05 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 08 Dec 2023 09:53:05 -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=1702047184; x=1702133584; bh=Uz BFYxHGgBVtEJGlUELGUayA7oNlbWLe/nYuyIkfrfU=; b=YcUsJNC7DGRPM5d8Xb oQbcXGr53VX91tZJxHqtZvCsulMkt0qMYKIUlfrWf1frke0PAtInofWdyQMY01gK Yn3gF+Yh95ZGlJeKR6mI5LG8m1K3kWiQeA1x5fUVdj+GUTOuRQ8dgN6HUTmZqm+M UcGWCb9f0bnRkRWtCX5wvBmSB8WWmQgYTYl2JsF5IZ0jcS1EyNW0LLIU1Xhl8f8C cACBHqfnNx1KiHoLmHGBOgtR+wkn/Bv5ryj0L73JuDmjVbCKIJDsAcdRgLgtxiTj v33yaj8wLd2Xy7Y5+Zzd+nW18eQZMH4iil+xJEkuGeMgPaIehzZvS5bAjaqPIvPH uYzA== 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=1702047184; x=1702133584; bh=UzBFYxHGgBVtE JGlUELGUayA7oNlbWLe/nYuyIkfrfU=; b=hr4BhDW8WlH8LhlrXbc0BKmNiCZ8I t67MzhEzUE3dQi/jNd79bhIV6hJF5soHV4oA9OpQsJRK/1+s+bz0OS8TDYvAGDOg eLBty6UshvsRHj1ncQOOvLN71mC8vprBDqp7MWpWodjqCIBBNDLWF7vT8zf8IOqX 4s+Of5Ns0nFamnIQrTXgiFhLCW9lJZ4pEWulO0vmZDl7L8lscn20qp2zXv3aTC83 8+CzdzjxxeBCK1a7FCnTtjUdG73Y1DlPFDUxTkmb8+GwOJOAPa9C/fnMAJyxi74F jwjKGN3fSLkTaPza1gBGAjKfQFxSWBwMy43rv9PQZ5lIFc6+Ia9FXFkzQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:03 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 3cb134f2 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:33 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:02 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 02/11] 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 Fri Dec 8 14:53:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485478 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="UfpFUwk5"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="FU0vdZIL" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 386964ED9 for ; Fri, 8 Dec 2023 06:53:11 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 549B03200A01; Fri, 8 Dec 2023 09:53:09 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 08 Dec 2023 09:53:09 -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=1702047188; x=1702133588; bh=9L LqehmDLTPnmdpLWFmrwJ3o8akTNT9TXYtBI0QXN7M=; b=UfpFUwk5CeXsqxkbzQ VYaGkK6j/dJK0D7Bo2DoTZ70ErHJxw+bxheAAokTKf/a6O0n3TMA9Zgz/zByql/z kDF7UL2RWCxZxTly2qgZLRKDEWaHGrO4inxiRagUd0tqUPn1lhpQSX0rc7rnQ2FU AM7sVhDPo98tpkscdX5hUmDos+6atvdIdjK+Tl/VLDS0VDzGtNULa2YIT1QiVee7 FK3wFUkoy9duholREzOg7iR9nVaojm51Pz+K4W2xBeBYniNsxF720p4E/hfw3dOZ bdCxOP2n5I57tNO3v/67iesI30iV6GTyU48LDPReTV21iXHBuVkV62O3mqZZRYKj jUlg== 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=1702047188; x=1702133588; bh=9LLqehmDLTPnm dpLWFmrwJ3o8akTNT9TXYtBI0QXN7M=; b=FU0vdZIL97y3FUMZeH5fTsv2lNt66 VO1l16Wtvj3YWhw6su8uYN3UZvb6ynKBWzT/1AVNb/FkdyMprQkSS9r5sXqsWmXH AU98QgZ7qT7WfTaRcnZBmRI9JJcjppybHAwAKfstuQosaSKoHuCG6FCMXUYt53Oz KBh9HvuIi77GBFrVZAWZxZKiVM16TKEH2QRqBugKjAeVbXeLvfetwx456D7SEPEA OosisZZGbCd2AGnprCMQG/dNeNH/KQpk6UX2TtG3s1u11jkxOyZGhL9epsDzjesh 1HrkYzupjox4xK65UpkE1+cjKMbMkmgAQGTywmpL4G3wQl71K6Gyf7lQQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:07 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id b9cbca9b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:37 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:06 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 03/11] reftable: handle interrupted writes Message-ID: <8c1d78b12b5b8d7c4770e627790336c442aef665.1702047081.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 Fri Dec 8 14:53: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: 13485475 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="vKrLDiEU"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="pnOXc4gZ" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23A15199E for ; Fri, 8 Dec 2023 06:53:14 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 7B0B03200A44; Fri, 8 Dec 2023 09:53:13 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Fri, 08 Dec 2023 09:53:13 -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=1702047193; x=1702133593; bh=Eq lcaFpix6RMcFAN5cvFjMyS3zs/JYMwp4Q9eTiC1Wc=; b=vKrLDiEUyhTShc+9tr AcNWDMMfSFZkvys8yfHv8oOh/6Vd3teQpw4C06C9e+PNlrJATHoZfj9oenGhgyJ9 GFkNDgTuLLTMmeTPpqc6f+pE0Mva9GShHmNUbNUHACJDmGwSjbXRlntEsVmGRxxS Q/ymyokQuvUNrC5bZG8sWfhc1p73KMCMg2mCdR7gkx6rbmZJIJN7CsAN2fsEEUyg y16qvn/xu7tJbnRMSlo3PZtZyhUhArGWB/8bLRIJkQ/ayTHHikdWQ0iTAUPfDtJK PxFleVWkQ9HJu9kR2+k27O1AEqoEWg107mOW+Vg1XaNJ8HngC+Fka2uvU27jmOZs /sDQ== 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=1702047193; x=1702133593; bh=EqlcaFpix6RMc FAN5cvFjMyS3zs/JYMwp4Q9eTiC1Wc=; b=pnOXc4gZmuqnIjwvCr34D4fuZvkv9 /MCRaT7d3WIPDCe30qzU4xlk0+nBUUYYOEh/c6AAtbvdjdDb9us0vQqE9eQyAAde KjJQ7oiLt4bkJLZvNK79rkLTbtBZVuqez4vdF4EkvYpTcJqWL/cMEMTtamoZUNl7 P4sNMhnuspwHeoBuowwCSX/zPo/3yrAp0hjZ+AahB+DazNfnyS1Iu+kNdPqHvXJG e89iBsaEXIbqAaAuzoGkrTuALOn/j0MaHYNLsIIjm9XxLO78BodRtP7753U2KBBO fz6V4romn0lCHp9Z4zhTFn3lWvk1dzdMW6WzeUfopTLiGABOYvQrkUQFQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:11 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 7f64b4ff (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:41 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:10 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 04/11] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction Message-ID: <8061b9d2fcb3e8c3d1fd641e705b9a8879e452f4.1702047081.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: 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 Fri Dec 8 14:53: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: 13485479 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="ib6OYtQa"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="mZX1L/Na" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EBB96524F for ; Fri, 8 Dec 2023 06:53:26 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id A183E3200A26; Fri, 8 Dec 2023 09:53:17 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 08 Dec 2023 09:53: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=1702047197; x=1702133597; bh=Sq A5850dZkMlElWI0onK5+fB8eDH3K5LlGb6P48nI0E=; b=ib6OYtQaPGqR2c3LCa 8BKF6Wne48NGDdQdhFQjoLBAFBBiIvJ4zKp9CDnH9ILzxJD4Cl55ty8Pscn6v6vr mT2l9Wh0Cg6tqOwFSU4/zaI+fVsf6RCL6bsk70q1+bKaDmxiKp0HRSBcHye61k0m KIoZmXARbCZI8gJYDOzm3mbrUbfoVmsLMOBI2dXRdL7HARdjVFc85hjfikXySocR p3rAVhodXoL2KyQHeeSz9Ag1cQo3BlXpQy9tC5i43ZmK9SK5AZd98IZDgWE7tNeZ AgpN0ZgekqsxveYCGVtBEMH+UJe3MoObPI/3kNxCPj9B50hDAR1cUaYUvnrnjXpv dfvA== 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=1702047197; x=1702133597; bh=SqA5850dZkMlE lWI0onK5+fB8eDH3K5LlGb6P48nI0E=; b=mZX1L/NaYOwgU/b7GOaQRzw+Ds/M7 u9u6A0uMKxemnQM1f4rOaW1df/4L4SZeaJw1b4QvfYpvopUJWjFWAvVbgMtdnJ9d OjL/s+L+1IrJz6SF4zDZjv4I4OuQKkCcd2fYIIyRgDQkdwF5olYg1oKqZ8H7cV7+ 9KpRFK7Hohai2NlVv1WjT5qZPeg0pnOwvMzPIo69Dr3FqqnATUoZS0sc49Pb0emH CEfY2niBS2ktW4TCO/xupqbIGzbimiZJkDoGe32dDdiDu1mScQxMlGcDc5JsOsZy rZdpM89N48D3zIroto16WRLbDcKSpJDqxa73m4/xrJwFoRCqT83MWiGQA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepvdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:16 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 82264f47 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:46 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:14 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 05/11] reftable/stack: perform auto-compaction with transactional interface Message-ID: <77b9ae8aa675dd96dd10f4a5369f1f994fa59939.1702047081.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 Fri Dec 8 14:53: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: 13485480 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="CfzIsWhX"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="P60Pjz7h" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EBD185251 for ; Fri, 8 Dec 2023 06:53:26 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id C86A73200A28; Fri, 8 Dec 2023 09:53:21 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 08 Dec 2023 09:53:22 -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=1702047201; x=1702133601; bh=0K B8yHQRfh7qieVzM8MgyP4GTYU6xovElQvFFjF5OmM=; b=CfzIsWhXiMpj+hQoKA aGyN/0yxn3fyZ40+2FcE0JykVV8uZ2XvYlngpERXNQufvjieFHde6tItBUe7ouuO ydm+pwHqH6cPgRMYiHD7Eb6UHM7DjwaR+g+YUY81VmVOEV6XfDbtUpgoz0Cd85qA hg+H9ZgxUqyMARC7wTsj9OEfYbQHmA6Jy7xWcTttFMxXDnEHdG50MBODK4e3OAGc 2bLuKIDUHo7jNNyiFtkjylzUsogkJ4OxsuIZ8FcrHOqe7XxcFWcjv2buy4NXeOkj yCxiQeSXl/++fb5IuGOFBSnWrrEyiCsLs+z5LILTLRJkefqQlmxoSsf8t4KG4B5P S+UQ== 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=1702047201; x=1702133601; bh=0KB8yHQRfh7qi eVzM8MgyP4GTYU6xovElQvFFjF5OmM=; b=P60Pjz7h/03Sd5qEf7CvNi9zRPEMn S3xNgd87uMGtefCi2fKCK0jC8mZUwFvQedmbgeM+dmsP3mlz6+wUsFK0VznfC4w1 YAcVygW3pPp3XAmCXb4l+fOUn5z4tMJMRl+hVLLMuCWfrd4Mq3pghx0gUkiaTAU5 uDPsaZ2fYzTluF90N2GIOKhF/ZrJLTDn4aXGGg/R2RKlRNzZBd6w4O8AU8xgi6Bw H9B5pebumnqljaHKncLqenH9+fqEld4ToHKNua0+1CrkygOoC54J7oXJAZMwi3hk 0250P5YjUr8d2SXfbctbhlRNQBpmWzF2agPOJl5fpruAoBhwsgGQ7oozg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:20 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 62057270 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:50 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:18 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 06/11] reftable/stack: reuse buffers when reloading stack 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: 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 Fri Dec 8 14:53:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485481 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="SckDeTvG"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="1mP+V/22" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4ED621FF6 for ; Fri, 8 Dec 2023 06:53:31 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id E600B3200A02; Fri, 8 Dec 2023 09:53:25 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Fri, 08 Dec 2023 09:53:26 -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=1702047205; x=1702133605; bh=2o QjgDlEqnI3GqOz/Huo1YNbwyRyQBA3lZvTIdOsB/E=; b=SckDeTvG4zes/JdfSY FcyMpoV8RtZlmMmdYnlvXeqvgyeLmswN1goBEwCX+QzEZt+j9aeFbjog4MA7jKif ECXdt1OyIIdDnKB9OSy0duGybxweHEW3EIsapX4KZ3dRbWya/wAPnuzoImwYFTaV XvJyHa8npaUaaRe1hsdNu1FjIe4xksNgVWyZtk/sSHr3S8+H/BVYOCWTr4yWgoAh g3F5uqXPmwgnRitq4HPvCSk3XphtYumHnpws1bQJEarCmLrTlzSpbGyFKN2q0bUl ZC5GAfLZjkFck0rgaRGmuoJnNaiDXQhch8sEGoMD9Fv9FGHu6R7mfFup73LfGKz/ V8VA== 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=1702047205; x=1702133605; bh=2oQjgDlEqnI3G qOz/Huo1YNbwyRyQBA3lZvTIdOsB/E=; b=1mP+V/22V9I4QvJyQshPSxAWodwBi QdoHODykpV3k96G0yDwOy/5pBchKjG+/cDsa3z7DZQ96WRmqpHcNPkkp+0Sb/PE6 EesObAYZ/M1qNrLDTDG9rVnk/5LMJiaXgypqPVblkil613lGqO6+R8W/xnR+6zEv uszOPh8Q7CYGTiV2TXy42eyQsN37uQIyA3FHMJSTus1qhTMsxwzN8dSn6giBYhQn EI+OkkSmMTVRhllAiQEPD2OqsTuX/S9Lh+6lqa2Saev7RP8WBE/bO5IlqTEv2h0N ai3+yRTFL6/b95Baog2m1ZEcwkkjTvKiP0TFO14mTGzGVyG73JAMtLzuA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:24 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 07a6a2da (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:54 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:23 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 07/11] reftable/stack: fix stale lock when dying 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: 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]); } From patchwork Fri Dec 8 14:53:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485482 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="sJjg2zj+"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="igqYCdye" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C55E9212D for ; Fri, 8 Dec 2023 06:53:31 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 16A663200A1E; Fri, 8 Dec 2023 09:53:31 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 08 Dec 2023 09:53:31 -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=1702047210; x=1702133610; bh=Zt xTvqpV4ITb7Bp9x0Pj9C61/47efdYtZ8Tp3tEfEWM=; b=sJjg2zj+TSFH0OBgEg ZN7Jbl5o6ywWWH+CS5uGWioMQ9268hK5Fix0P/eUtvkxjRugLeUJpYiQFE4Frvi0 lRzu1E4oOXu/jVffEeaXN+yXNTbhD3kOm9OTHl8Bz6wzCx/15P0+nVMOZg/4opi+ DKHe9Rg2rDfG0RixFZtGvuyXuM2ntN7QI69YgW3vAar/sBYYz1GIVBhmNmMaBKmB TxX6VQ04vECht93FaU2TS0uYFJ6Xk0pwFsVZ0Ee1DA4lY6N+NZLNFsQnU2g4dyLP AJrYktambczX1Lj8ZfysR7GGcxFqtKPRH03jMRQr1vPzXd6H8FdufFM6/67KLQgS eQJg== 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=1702047210; x=1702133610; bh=ZtxTvqpV4ITb7 Bp9x0Pj9C61/47efdYtZ8Tp3tEfEWM=; b=igqYCdyekeRQwZLmZXLI8OzKhAQp8 e9dXWzoNiia+CHW8VdMo+CVygOMcsXSWegaElIv013ASuaplv/5L/91n2mddFQzs qOAZmggvyLeQ8PvA5R6tkT0uy6DvAV0mnmIg+jOv++3dsqCwpz/NXWN/xVYmNH4h 8Rv4fwNYPZBXfecfkoOoQ0k3003U1gWnoH+dvzUFRGNA8/Y3+XiFSdZqbOskqR6m VBVM+o5+yZiyf6BJ2+U2XWGgKPCehP7pRVzTo6HaLsL4V2pMPfWKWPvU07ZTOuB2 Z4ZJCFiQz1Erlm0XKirxlRWSUqfuD1v0bJghwDyIF3Nkaq4vOebLjTn8g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepgeevtdffhedvheehgeeuuefffeetiedtfeehjeehteevieehkeekieefhfdvveeh necuffhomhgrihhnpehuphgurghtvgdrnhgvfienucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:29 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 38a14f24 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:51:58 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:27 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 08/11] reftable/stack: fix use of unseeded randomness 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: When writing a new reftable stack, Git will first create the stack with a random suffix so that concurrent updates will not try to write to the same file. This random suffix is computed via a call to rand(3P). But we never seed the function via srand(3P), which means that the suffix is in fact always the same. Fix this bug by using `git_rand()` instead, which does not need to be initialized. While this function is likely going to be slower depending on the platform, this slowness should not matter in practice as we only use it when writing a new reftable stack. Signed-off-by: Patrick Steinhardt --- reftable/readwrite_test.c | 6 +++--- reftable/stack.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c index 469ab79a5a..278663f22d 100644 --- a/reftable/readwrite_test.c +++ b/reftable/readwrite_test.c @@ -141,8 +141,8 @@ static void test_log_buffer_size(void) */ uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ]; for (i = 0; i < GIT_SHA1_RAWSZ; i++) { - hash1[i] = (uint8_t)(rand() % 256); - hash2[i] = (uint8_t)(rand() % 256); + hash1[i] = (uint8_t)(git_rand() % 256); + hash2[i] = (uint8_t)(git_rand() % 256); } log.value.update.old_hash = hash1; log.value.update.new_hash = hash2; @@ -320,7 +320,7 @@ static void test_log_zlib_corruption(void) }; for (i = 0; i < sizeof(message) - 1; i++) - message[i] = (uint8_t)(rand() % 64 + ' '); + message[i] = (uint8_t)(git_rand() % 64 + ' '); reftable_writer_set_limits(w, 1, 1); diff --git a/reftable/stack.c b/reftable/stack.c index 2f1494aef2..95963f67a2 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -434,7 +434,7 @@ int reftable_stack_add(struct reftable_stack *st, static void format_name(struct strbuf *dest, uint64_t min, uint64_t max) { char buf[100]; - uint32_t rnd = (uint32_t)rand(); + uint32_t rnd = (uint32_t)git_rand(); snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x", min, max, rnd); strbuf_reset(dest); From patchwork Fri Dec 8 14:53:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485483 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="hKQVhEQl"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="P/P7SSAX" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A3CC92695 for ; Fri, 8 Dec 2023 06:53:36 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 7BC363200A33; Fri, 8 Dec 2023 09:53:35 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 08 Dec 2023 09:53:35 -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=1702047215; x=1702133615; bh=8F 0/oFQqHSzKyZi5t1vp1b2t9e42ME5tkwAbvPgw+vw=; b=hKQVhEQl8wv1+rfMeJ +YJimozOw8sWPBgclqB26Ad/VEoAnOVCNGgSCqzvM4JNelOu/MAZs4IPFO+xghFq fNnXne8xuVLVo0c8+lPQ91GDZCMtunehDlhp3IKt/yLkhQK8EYWfei7SoQqbJjqN T5kx9QOWcNx/8rsICqp/kV4GH004MoGUkvdRv7L6GhQBWG6hcNzYwhomU+vvHJhS zMrupGzeIddXBghw78N9fjN/lg5EMBWr1qhC8EaG0pWw3OoflnWAq0SZeWD0+Rqj xVR+rEwGTd5jPyB0TWwlJ2V/EgH4KzjSC6wCWlZ/RIEnlhSdL2p1Cj1GW2zSde8S cCpg== 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=1702047215; x=1702133615; bh=8F0/oFQqHSzKy Zi5t1vp1b2t9e42ME5tkwAbvPgw+vw=; b=P/P7SSAXgoeaADKZbEgWrqAQMpCJe puHK+sAIaaXiERLDw6xY94ZKxrLJweboKevuil80qcH1ExANup4uIWjDmiZISK0U SzgvlPC5Zo0mDqBbrIr1OdzQQEmqrnQGRUNLSoTnmfQfjm3vaufftqyxqXrRmNLB tvBMAB8Dp/jBe9YOKYAfIsl9a2Ee3BIhBwtkC6uQ8vJG4Vg6eCi/HymO9zXVkYTK P8TswUEY7lFPEehdXDBXYGFfXFNSt3BKKb3X0Sa5W2MSFxcFAQ/f/PT66yZnv2co ZYKEkSwOOa2mWxGF8hsIr4ijeZg/UQxlxnfLvcDg/akB7ub9Sm7ZrhmCA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:33 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6dcaad72 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:52:02 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:31 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 09/11] reftable/merged: reuse buffer to compute record keys 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: 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 | 31 ++++++++++++++++--------------- reftable/merged.h | 2 ++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/reftable/merged.c b/reftable/merged.c index 5ded470c08..556bb5c556 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -52,6 +52,8 @@ static void merged_iter_close(void *p) reftable_iterator_destroy(&mi->stack[i]); } reftable_free(mi->stack); + strbuf_release(&mi->key); + strbuf_release(&mi->entry_key); } static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi, @@ -85,7 +87,6 @@ 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 pq_entry entry = { 0 }; int err = 0; @@ -105,33 +106,31 @@ static int merged_iter_next_entry(struct merged_iter *mi, such a deployment, the loop below must be changed to collect all entries for the same key, and return new the newest one. */ - reftable_record_key(&entry.rec, &entry_key); + reftable_record_key(&entry.rec, &mi->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); + reftable_record_key(&top.rec, &mi->key); - cmp = strbuf_cmp(&k, &entry_key); - strbuf_release(&k); - - if (cmp > 0) { + cmp = strbuf_cmp(&mi->key, &mi->entry_key); + 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(&mi->entry_key); + strbuf_release(&mi->key); + return err; } static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec) @@ -248,6 +247,8 @@ static int merged_table_seek_record(struct reftable_merged_table *mt, .typ = reftable_record_type(rec), .hash_id = mt->hash_id, .suppress_deletions = mt->suppress_deletions, + .key = STRBUF_INIT, + .entry_key = STRBUF_INIT, }; int n = 0; int err = 0; diff --git a/reftable/merged.h b/reftable/merged.h index 7d9f95d27e..d5b39dfe7f 100644 --- a/reftable/merged.h +++ b/reftable/merged.h @@ -31,6 +31,8 @@ struct merged_iter { uint8_t typ; int suppress_deletions; struct merged_iter_pqueue pq; + struct strbuf key; + struct strbuf entry_key; }; void merged_table_release(struct reftable_merged_table *mt); From patchwork Fri Dec 8 14:53: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: 13485484 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="GQsrfgFw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="tdNxEhDQ" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6886A2115 for ; Fri, 8 Dec 2023 06:53:40 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id A6BED3200A2F; Fri, 8 Dec 2023 09:53:39 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 08 Dec 2023 09:53:39 -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=1702047219; x=1702133619; bh=0s FTJ9aXTpCWq74275Rcgq/s/ZG9uvwafIGo3u7CVP8=; b=GQsrfgFw7oMuV8tTif ppKOAOojLSenkHhDlCFp51cfIgQ6Z/w5LjLN47nf+oJ3o35mh2xMKzmng+YNBGLm Rz7wulEOSlhOQCyPjGaPJCSOKU5eYs75pNSk7qFh+Xtrd5sp9L4BcP1h1iyTq2GI hLw28dtS9Z5e9hKjm0KGDpcswDxiiYyEswNCcqgmrF84FFNh/vpxLovxVwqlSF9V bWx8YNhXib4iINzGez23DZBUg7RFPNUVZogTLQQRfAc3e0P9x9uIUmCjFi3z1cA2 Fm2Ar8gU7hlILehgHvxLuu13sCZqzkU+vPx8/LPiOcxRjIsseI3AfrD0kCiJCTaa 2ftg== 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=1702047219; x=1702133619; bh=0sFTJ9aXTpCWq 74275Rcgq/s/ZG9uvwafIGo3u7CVP8=; b=tdNxEhDQRBQri3W1Oyqcd90keNbJR uqI3fD5f7xwmog8FZFNx1oryZ0pY0VA0k/ObvxTQD+PYDtFrVUX3MpYjxHL2qTZ7 XFyBoG7lpdLW8SRPmoGl9ydWxc9xJphuFFe4pUyqdRdMnbNWEbVp5eNvrfeO0VJB r4MwdM2981QapZRLyA2rWEzinpIAoBrO9TzESs1eOrvixcKEyV81QOAjpmW75rDk jqCzHbWXBtk0nHaTwuM11nfaob9DihGA7etL0ftdNoE7Y4H/tvw7k2pT1U+ePDxo iD4qAhchnFcoEnekojeIwt5UIURIq26mi1lx8ghKlj8BIcHpwwc3Ol8+A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:38 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id da07f292 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:52:07 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:35 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 10/11] reftable/block: introduce macro to initialize `struct block_iter` 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 a bunch of locations where we initialize members of `struct block_iter`, which makes it harder than necessary to expand this struct to have additional members. Unify the logic via a new `BLOCK_ITER_INIT` macro that initializes all members. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 4 +--- reftable/block.h | 4 ++++ reftable/block_test.c | 4 ++-- reftable/iter.h | 8 ++++---- reftable/reader.c | 7 +++---- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 34d4d07369..8c6a8c77fc 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -389,9 +389,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, struct reftable_record rec = reftable_new_record(block_reader_type(br)); struct strbuf key = STRBUF_INIT; int err = 0; - struct block_iter next = { - .last_key = STRBUF_INIT, - }; + struct block_iter next = BLOCK_ITER_INIT; int i = binsearch(br->restart_count, &restart_key_less, &args); if (args.error) { diff --git a/reftable/block.h b/reftable/block.h index 87c77539b5..51699af233 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -86,6 +86,10 @@ struct block_iter { struct strbuf last_key; }; +#define BLOCK_ITER_INIT { \ + .last_key = STRBUF_INIT, \ +} + /* initializes a block reader. */ int block_reader_init(struct block_reader *br, struct reftable_block *bl, uint32_t header_off, uint32_t table_block_size, diff --git a/reftable/block_test.c b/reftable/block_test.c index cb88af4a56..c00bbc8aed 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -32,7 +32,7 @@ static void test_block_read_write(void) int i = 0; int n; struct block_reader br = { 0 }; - struct block_iter it = { .last_key = STRBUF_INIT }; + struct block_iter it = BLOCK_ITER_INIT; int j = 0; struct strbuf want = STRBUF_INIT; @@ -87,7 +87,7 @@ static void test_block_read_write(void) block_iter_close(&it); for (i = 0; i < N; i++) { - struct block_iter it = { .last_key = STRBUF_INIT }; + struct block_iter it = BLOCK_ITER_INIT; strbuf_reset(&want); strbuf_addstr(&want, names[i]); diff --git a/reftable/iter.h b/reftable/iter.h index 09eb0cbfa5..47d67d84df 100644 --- a/reftable/iter.h +++ b/reftable/iter.h @@ -53,10 +53,10 @@ struct indexed_table_ref_iter { int is_finished; }; -#define INDEXED_TABLE_REF_ITER_INIT \ - { \ - .cur = { .last_key = STRBUF_INIT }, .oid = STRBUF_INIT, \ - } +#define INDEXED_TABLE_REF_ITER_INIT { \ + .cur = BLOCK_ITER_INIT, \ + .oid = STRBUF_INIT, \ +} void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it, struct indexed_table_ref_iter *itr); diff --git a/reftable/reader.c b/reftable/reader.c index b4db23ce18..9de64f50b4 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -224,10 +224,9 @@ struct table_iter { struct block_iter bi; int is_finished; }; -#define TABLE_ITER_INIT \ - { \ - .bi = {.last_key = STRBUF_INIT } \ - } +#define TABLE_ITER_INIT { \ + .bi = BLOCK_ITER_INIT \ +} static void table_iter_copy_from(struct table_iter *dest, struct table_iter *src) From patchwork Fri Dec 8 14:53:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13485485 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="W9Nyo+9G"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="C7cE9wxd" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D95F226A9 for ; Fri, 8 Dec 2023 06:53:44 -0800 (PST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id 25BD93200A26; Fri, 8 Dec 2023 09:53:44 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Fri, 08 Dec 2023 09:53:44 -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=1702047223; x=1702133623; bh=MR 8/vdeKYlYQpksNgtUNJ5uwcpcroH1Gk4ksTnYEmuM=; b=W9Nyo+9GmALbV7PkKG tdsqY+QjR20tXmjDMjtX6Z+M/w9F0E/uYA2Wt5K6UTUtqgqKN5GdZPnVba+Fhtqf ABb5Qegx+XI1FnvNfgIwVg2zRpiynhooU2vobDlalb3obubMUY3oJismMvGxtQx/ o9piGmuli744FdkiXkfWQpIE0Ox0WlO6x2hPfm4ey0J4Hkc9Hvalt1g0x5PU8cbf i6c84VFriHS++hXi+k1jon5/OtaNkwzZ9u+VgkWa3xbe/8mGzcakYicBkrKOMYBn g14QpnbmXXrZ3qiU6ZMiy5zjU1xFMG8MKhVGfj06Ql7f1Y6hXabsCxdqia+cu0/E N01w== 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=1702047223; x=1702133623; bh=MR8/vdeKYlYQp ksNgtUNJ5uwcpcroH1Gk4ksTnYEmuM=; b=C7cE9wxdxCKFOZ/VDbMouxcCZ4So0 yGQHOkygBNksb0tSMeoeW+drop9TL6RL7gBzXGe91L/QR9Bl4Yy6IgOVWlz6wR43 nOVIU5RPH1oa7q2PickN1vgRyAoGr7wbH2QQ4sp9qP4qHCrDnfu5r97wtvdwEC5M BJEhYtDL7Ootese/DFnrmqCE3R/yq9NFwaUhVzzJgYNO/8AgJ5BjSy/mcGtRk0J6 eS8W4dRGno2JYOwPeHkQsByMbbxgTqmoHFMq2PHv5d/Pu9qamDxHxGeo54Wnp1dg 8kyBGjlnt0Y4r/s0gkL6YWsFSTqxuEjCFd8Iq5oR51W8ed/uNioJdMH5w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudekiedgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 8 Dec 2023 09:53:42 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id be602c60 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 8 Dec 2023 14:52:11 +0000 (UTC) Date: Fri, 8 Dec 2023 15:53:40 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Jonathan Nieder Subject: [PATCH v2 11/11] reftable/block: reuse buffer to compute record keys Message-ID: <02b11f3a80608ba8748a0d0e2294f432e02464e5.1702047081.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 block iterator 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. Refactor the code to reuse the buffer. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 19 ++++++++----------- reftable/block.h | 2 ++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 8c6a8c77fc..1df3d8a0f0 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -323,30 +323,28 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec) .len = it->br->block_len - it->next_off, }; struct string_view start = in; - struct strbuf key = STRBUF_INIT; uint8_t extra = 0; int n = 0; if (it->next_off >= it->br->block_len) return 1; - n = reftable_decode_key(&key, &extra, it->last_key, in); + n = reftable_decode_key(&it->key, &extra, it->last_key, in); if (n < 0) return -1; - if (!key.len) + if (!it->key.len) return REFTABLE_FORMAT_ERROR; string_view_consume(&in, n); - n = reftable_record_decode(rec, key, extra, in, it->br->hash_size); + n = reftable_record_decode(rec, it->key, extra, in, it->br->hash_size); if (n < 0) return -1; string_view_consume(&in, n); strbuf_reset(&it->last_key); - strbuf_addbuf(&it->last_key, &key); + strbuf_addbuf(&it->last_key, &it->key); it->next_off += start.len - in.len; - strbuf_release(&key); return 0; } @@ -377,6 +375,7 @@ int block_iter_seek(struct block_iter *it, struct strbuf *want) void block_iter_close(struct block_iter *it) { strbuf_release(&it->last_key); + strbuf_release(&it->key); } int block_reader_seek(struct block_reader *br, struct block_iter *it, @@ -387,7 +386,6 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, .r = br, }; struct reftable_record rec = reftable_new_record(block_reader_type(br)); - struct strbuf key = STRBUF_INIT; int err = 0; struct block_iter next = BLOCK_ITER_INIT; @@ -414,8 +412,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, if (err < 0) goto done; - reftable_record_key(&rec, &key); - if (err > 0 || strbuf_cmp(&key, want) >= 0) { + reftable_record_key(&rec, &it->key); + if (err > 0 || strbuf_cmp(&it->key, want) >= 0) { err = 0; goto done; } @@ -424,8 +422,7 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, } done: - strbuf_release(&key); - strbuf_release(&next.last_key); + block_iter_close(&next); reftable_record_release(&rec); return err; diff --git a/reftable/block.h b/reftable/block.h index 51699af233..17481e6331 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -84,10 +84,12 @@ struct block_iter { /* key for last entry we read. */ struct strbuf last_key; + struct strbuf key; }; #define BLOCK_ITER_INIT { \ .last_key = STRBUF_INIT, \ + .key = STRBUF_INIT, \ } /* initializes a block reader. */