From patchwork Mon Aug 5 13:07:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753603 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4FD6A155389 for ; Mon, 5 Aug 2024 13:07:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863272; cv=none; b=FGK2C0YkahE8vvG05Drw2LtQZwIb5lzWpLiAtYYuzT7Ci28bYEqC6+wuQMxld7ACQWjWPX5gS6ccnuCsd70CGgfxTAeaGaQdg7gMDN5ipD32//5XgpA+Ov3y8pA6fqQRnDW9ZjxXSwmZe8Mi3GIWgRnvMd4k1pZiQ4oayYQnMTs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863272; c=relaxed/simple; bh=qX/IKyWt4Lv5Yq2nDekfd8V2Byd0s/DJpfYdVYDT8Y8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kEe1FuCVyYg7b3B5WPKKsVl9OQSbUBFiqHzoPOlarQfvQuvVbJujcnXRsLiKW8Fq9ESCtwJh1JlIWgBjJ/2N4xOHZDEZIHqWhPzV2LsEs51WbgwN7XODy8/7isfdCX2zH8Tzg3QPccgjFjaKtxM9O0SJsfIJAkznHHwaKdjDtAU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=N+CMLSLY; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ABZAMkUa; arc=none smtp.client-ip=103.168.172.145 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="N+CMLSLY"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ABZAMkUa" Received: from compute8.internal (compute8.nyi.internal [10.202.2.227]) by mailfout.nyi.internal (Postfix) with ESMTP id 6D5B1138FC79; Mon, 5 Aug 2024 09:07:50 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute8.internal (MEProxy); Mon, 05 Aug 2024 09:07:50 -0400 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:subject :subject:to:to; s=fm3; t=1722863270; x=1722949670; bh=4zs+Q2H/od s5lCCBD1XSTAQPJda7d+/oYBUpxcU5X30=; b=N+CMLSLYtOkyg4C2NLuomP6h8N gwSfBR9jNNGZUd8pNb9HJi77PK1S9XSM+B01UxHwQ6HOq33Mns1ORYSmXPAstdZp GS2IkXSLGW6v+9caT/Ei5Jl6jajmXqi5GRG9GRTa2kTQ2XIePDgYN6o1AbX9Gg81 /dqzU/fpLvLK0trrXspC0iznxZQHYuEBmqLH9IZVbmPCMDxi0pt0/wnaEDKHGQOc yRGP2jiTh28XlsWPX/bPS5Y2xfhreYbVU8sZnUdXks9Km81pGwJo9vIko6/WhRjJ wos9+GhnbBLF/a+l5xVonZU0k9IhDAAnik2gCCwykzJReBZxSUbP1ZT4lL+Q== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863270; x=1722949670; bh=4zs+Q2H/ods5lCCBD1XSTAQPJda7 d+/oYBUpxcU5X30=; b=ABZAMkUaG0QPlfqrEBUZm4niB75IvvasmVTgshm2GmlN Zztl7XKBJ6Wern0TSioY/rCdYf0b16bTDCYjVUJxzT2IcmbNJ9UUJ9Qt+wLC27hk CHlmX9SqLwwX/wjZii5JOad0UsNkBg98kD+JAibDLQFtkAiRqHCBhHfbXi3rvy5V amx3oX1my7MxzCsp9PWOhXfEUHNdlqv0kHpGi2j+FkW9IhRKhwjbvEdbc0ZyXTHh /23tn+piyx7wdqLWj2ShgUfnzuTbh3iWc4blQIpidnUClbalwj+xpOezEK2VzCLb ilLRgN1sQQQUQvgzeo4Llc/qdoNMsqmj9CBc+82P0Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeivdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:07:49 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 66ff5cb9 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:07:48 +0000 (UTC) Date: Mon, 5 Aug 2024 15:07:47 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 1/9] reftable/stack: refactor function to gather table sizes Message-ID: <6d2b54ba8e448eaa2584976e1fe73c798842833f.1722862822.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: Refactor the function that gathers table sizes to be more idiomatic. For one, use `REFTABLE_CALLOC_ARRAY()` instead of `reftable_calloc()`. Second, avoid using an integer to iterate through the tables in the reftable stack given that `stack_len` itself is using a `size_t`. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 737591125e..ba8234b486 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1305,14 +1305,15 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n, static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st) { - uint64_t *sizes = - reftable_calloc(st->merged->stack_len, sizeof(*sizes)); int version = (st->opts.hash_id == GIT_SHA1_FORMAT_ID) ? 1 : 2; int overhead = header_size(version) - 1; - int i = 0; - for (i = 0; i < st->merged->stack_len; i++) { + uint64_t *sizes; + + REFTABLE_CALLOC_ARRAY(sizes, st->merged->stack_len); + + for (size_t i = 0; i < st->merged->stack_len; i++) sizes[i] = st->readers[i]->size - overhead; - } + return sizes; } From patchwork Mon Aug 5 13:07:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753604 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64D0518026 for ; Mon, 5 Aug 2024 13:07:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863277; cv=none; b=ZZ0WABxTfaI7YKOYIHmdP9dRQYp4I0PLFdbHcdNQdAbGUTsfvFUBGNqooDcqBHjZHDj/WaLmyE0u/z0PpuILQcOfQRywSJwRy3nn0jGUmffVDXFhW+/HalDjK+XINQQSo4Q0S0oCzZq39rI0Lfl4ePEyPYK6WhrFyb2r9g33jvE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863277; c=relaxed/simple; bh=XEWq3KR9I/V5/i/SDtdq4W8BZ2oOmhqqTWQGG5klV20=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=f8IRRGgWS/6lE/ME/jwfrOh7EMW3qoP2hsy2tD23IsLTrFiQaOSj15RSA7pSx2bgzjoOdP+fI0M2qYs5DftQC3Vyll8DV3GGVld+FOdTqGsXmoesZya80hUewU8EckK34LgUS5VXquDkpwUSS/pqYkbMEjpagtKgQ6BA2SSW9Ro= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=R13E3kpB; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Qs/dNvee; arc=none smtp.client-ip=103.168.172.145 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="R13E3kpB"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Qs/dNvee" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 7F599138FC9F; Mon, 5 Aug 2024 09:07:54 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 05 Aug 2024 09:07:54 -0400 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:subject :subject:to:to; s=fm3; t=1722863274; x=1722949674; bh=rBvmoj3Swq YFoQXdXyqaVU73L9bpPx2rG2VK8oiciPQ=; b=R13E3kpBgWTc8uOJgcJNFrOzMl Jev9JCtejiTDhOpV1xpxrV3WnRXpJeqKKYFJuDggSK8BRa6HwME9OgaWn3Ycgd/x tB9S8LieTMcvBTAKGWr9O0R6WpHd87oXuN2NcV30hhIodCY0h7aeMqyt7gUL7ygH w/fi2W/phZFShNSp0sl9vo7ER5blu4wS0JwWpepQ3slfrG2y25JQ82kGXYZfl1iP VU0z7fuZuoBd+Nvwa2wkWmG5KGL7CRAvPb9357hHdaF1vP99FM8Dmz0wh/VWEX0n c3qijhXT4GM9FNVSRfLUGPEPpGKjOWozUDG8SICE20bxLWwjYDyy0nnR2RFQ== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863274; x=1722949674; bh=rBvmoj3SwqYFoQXdXyqaVU73L9bp Px2rG2VK8oiciPQ=; b=Qs/dNveet+GWwrE5ZTtZA1z7rLVMytKSOi+h+jCWqv3p PkFUxth0lWW1IMXdLBvTx4IZlHaRrWKY+gXNoWgY69g2KECeDZql4vG9HOe86Zz2 Fdt++UELTWFHQ3h39IRL3WIoBXVQX13wOGvd1TmcWzJJD2wZYOHItpCkpDji+qjV nQs10eNJ67aKcSgVf2Dp/D5HXZkgtpxDlIdfrjwlykZfrtVfVsSm2yhyN/tQVPZu 0ftraWWA5MWGiTQO9BjLzxMCiT+PtMcws9C2HQiXKAOmRvxusvSCm5CW9/TMhOxt TbgFME453j2syMy6iUjXPr5MJXxxHbdGlCvbOuTt3g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:07:53 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1d0a281f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:07:53 +0000 (UTC) Date: Mon, 5 Aug 2024 15:07:51 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 2/9] reftable/stack: extract function to setup stack with N tables 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: We're about to add two tests, and both of them will want to initialize the reftable stack with a set of N tables. Introduce a new function that handles this and refactor existing tests that use such a setup to use it. Note that this changes the exact records contained in the preexisting tests. This is fine though as we only care about the shape of the stack here, not the shape of each table. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 60 ++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index e3c11e6a6e..61dddf7f69 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -109,6 +109,30 @@ static int write_test_ref(struct reftable_writer *wr, void *arg) return reftable_writer_add_ref(wr, ref); } +static void write_n_ref_tables(struct reftable_stack *st, + struct reftable_write_options *opts, + size_t n) +{ + struct strbuf buf = STRBUF_INIT; + int err; + + for (size_t i = 0; i < n; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_VAL1, + }; + + strbuf_addf(&buf, "refs/heads/branch-%04u", (unsigned) i); + ref.refname = buf.buf; + set_test_hash(ref.value.val1, i); + + err = reftable_stack_add(st, &write_test_ref, &ref); + EXPECT_ERR(err); + } + + strbuf_release(&buf); +} + struct write_log_arg { struct reftable_log_record *log; uint64_t update_index; @@ -916,25 +940,11 @@ static void test_reftable_stack_compaction_concurrent(void) struct reftable_write_options opts = { 0 }; struct reftable_stack *st1 = NULL, *st2 = NULL; char *dir = get_tmp_dir(__LINE__); - int err, i; - int N = 3; + int err; err = reftable_new_stack(&st1, dir, &opts); EXPECT_ERR(err); - - for (i = 0; i < N; i++) { - char name[100]; - struct reftable_ref_record ref = { - .refname = name, - .update_index = reftable_stack_next_update_index(st1), - .value_type = REFTABLE_REF_SYMREF, - .value.symref = (char *) "master", - }; - snprintf(name, sizeof(name), "branch%04d", i); - - err = reftable_stack_add(st1, &write_test_ref, &ref); - EXPECT_ERR(err); - } + write_n_ref_tables(st1, &opts, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); @@ -965,25 +975,11 @@ static void test_reftable_stack_compaction_concurrent_clean(void) struct reftable_write_options opts = { 0 }; struct reftable_stack *st1 = NULL, *st2 = NULL, *st3 = NULL; char *dir = get_tmp_dir(__LINE__); - int err, i; - int N = 3; + int err; err = reftable_new_stack(&st1, dir, &opts); EXPECT_ERR(err); - - for (i = 0; i < N; i++) { - char name[100]; - struct reftable_ref_record ref = { - .refname = name, - .update_index = reftable_stack_next_update_index(st1), - .value_type = REFTABLE_REF_SYMREF, - .value.symref = (char *) "master", - }; - snprintf(name, sizeof(name), "branch%04d", i); - - err = reftable_stack_add(st1, &write_test_ref, &ref); - EXPECT_ERR(err); - } + write_n_ref_tables(st1, &opts, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); From patchwork Mon Aug 5 13:07:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753605 Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 826E718026 for ; Mon, 5 Aug 2024 13:08:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863282; cv=none; b=fICY67qCCseqfBNFU/e/Fsfh4twEZhx+qrhunce7iwutyowUeL71mkhed6TPNexdAZvGE+ujj0ADqb21tQ1kZ+OpIwOQUTq+83cLuphfd4xQfR4QJzSZDgnNHU5CiYXfnQhAgO2hQyGD4H9VeiHDBM8LFStTL+nNRHct9vJXwzo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863282; c=relaxed/simple; bh=8jMKZE9hxTjupKXxZzcGKPL2tThZORtqEHHQtd1fu+8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VHhUl3dV67c7VVKFPLgIhN2zCYvT0XfwzyZ+/fyAL5mO6VkcDIeUlN+tEQll2PcHjzEKraNdiV5fhT5t3aUmt5GwXoX05lB8oaNl6bl5fcYDSjs14igLOxlavrM0ESJ/ZZpGpQNRuCd8mitS/MSNypJ0GCwNvh13xxSo8XgO/8Y= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=k+IWnkmm; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=XKshQe/f; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="k+IWnkmm"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="XKshQe/f" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id A1F181151B3A; Mon, 5 Aug 2024 09:07:59 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Mon, 05 Aug 2024 09:07:59 -0400 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:subject :subject:to:to; s=fm3; t=1722863279; x=1722949679; bh=CPCDINA6iX Bp0WKVkmowxYemInt8WcxhsrBxbDkilJ4=; b=k+IWnkmmfPywAiE8uzFw3WMOta KsuL/DJl9xTh87QGoNsisNRsOyXGMGp+bqkd+p33IqzHdttiyFXO8unC1QypE33r 7x2T2iyeFVb3d0MXKMmfzw3tQ682wFDc7lHh8IPz17PcuWtY7RBiuNJb05u2XnHY XrPFHUlmiYLdPvMrNNqCR2dF76pNQbsqzs7vuXX/lFr/7AMjqrnuNmufo8YtBycB UxRg12byR5NWGWQweztZnMQTzPrDYjc/OLoICn1bz1VZG+FE9CazdZeICm61ONBA 93ISdtZ3y9yqQXHWnlGTK/LnCDpsrm4MgZtYtSryH3MfkkGMByrld26sTY/Q== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863279; x=1722949679; bh=CPCDINA6iXBp0WKVkmowxYemInt8 WcxhsrBxbDkilJ4=; b=XKshQe/fPyZg1kqRwK+ccFvRlyRPWHz/d1GKHVJYkMwV Ru8aYpnFI/1evxci+8WpKgEqGIy17xQrs3ntoaTEZFFlnlF9owH1oTT6qb2s3PsF RdpIX8zd/FtyC8AVDZNu6TnDQZ1JHt35v4zaJFv/wl1f3ymZuNWswMJbUJoPyGNt bz9BfIIwHjTGtzqQ8tjlMXYsBB0l+UpO3aq0i948KQvF7zVTB484G5vdORsZlxBS C3lY2KtlyChyjrTXmaBWHorWV134Tami/5KW7Tivym23ilwiS8Lcauuv603gYAdT XJzw8HcC3gDH1+3SqShDmT2FnDn26SURRVQxcLWPjg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:07:58 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 16416d7e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:07:57 +0000 (UTC) Date: Mon, 5 Aug 2024 15:07:56 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 3/9] reftable/stack: test compaction with already-locked tables Message-ID: <63e64c8d82783b5d3fc8db189a29d69c844f5465.1722862822.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: We're lacking test coverage for compacting tables when some of the tables that we are about to compact are locked. Add two tests that exercise this, one for auto-compaction and one for full compaction. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 77 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 61dddf7f69..46d376026b 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -887,6 +887,45 @@ static void test_reftable_stack_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_auto_compaction_with_locked_tables(void) +{ + struct reftable_write_options opts = { + .disable_auto_compact = 1, + }; + struct reftable_stack *st = NULL; + struct strbuf buf = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + + write_n_ref_tables(st, &opts, 5); + EXPECT(st->merged->stack_len == 5); + + /* + * Given that all tables we have written should be roughly the same + * size, we expect that auto-compaction will want to compact all of the + * tables. Locking any of the tables will keep it from doing so. + */ + strbuf_reset(&buf); + strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[2]->name); + write_file_buf(buf.buf, "", 0); + + /* + * Ideally, we'd handle the situation where any of the tables is locked + * gracefully. We don't (yet) do this though and thus fail. + */ + err = reftable_stack_auto_compact(st); + EXPECT(err == REFTABLE_LOCK_ERROR); + EXPECT(st->stats.failures == 1); + EXPECT(st->merged->stack_len == 5); + + reftable_stack_destroy(st); + strbuf_release(&buf); + clear_dir(dir); +} + static void test_reftable_stack_add_performs_auto_compaction(void) { struct reftable_write_options opts = { 0 }; @@ -935,6 +974,42 @@ static void test_reftable_stack_add_performs_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_compaction_with_locked_tables(void) +{ + struct reftable_write_options opts = { + .disable_auto_compact = 1, + }; + struct reftable_stack *st = NULL; + struct strbuf buf = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + + write_n_ref_tables(st, &opts, 3); + EXPECT(st->merged->stack_len == 3); + + /* Lock one of the tables that we're about to compact. */ + strbuf_reset(&buf); + strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[1]->name); + write_file_buf(buf.buf, "", 0); + + /* + * Compaction is expected to fail given that we were not able to + * compact all tables. + */ + err = reftable_stack_compact_all(st, NULL); + EXPECT(err == REFTABLE_LOCK_ERROR); + /* TODO: this is wrong, we should get notified about the failure. */ + EXPECT(st->stats.failures == 0); + EXPECT(st->merged->stack_len == 3); + + reftable_stack_destroy(st); + strbuf_release(&buf); + clear_dir(dir); +} + static void test_reftable_stack_compaction_concurrent(void) { struct reftable_write_options opts = { 0 }; @@ -1012,9 +1087,11 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_add); RUN_TEST(test_reftable_stack_add_one); RUN_TEST(test_reftable_stack_auto_compaction); + RUN_TEST(test_reftable_stack_auto_compaction_with_locked_tables); RUN_TEST(test_reftable_stack_add_performs_auto_compaction); RUN_TEST(test_reftable_stack_compaction_concurrent); RUN_TEST(test_reftable_stack_compaction_concurrent_clean); + RUN_TEST(test_reftable_stack_compaction_with_locked_tables); RUN_TEST(test_reftable_stack_hash_id); RUN_TEST(test_reftable_stack_lock_failure); RUN_TEST(test_reftable_stack_log_normalize); From patchwork Mon Aug 5 13:08:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753606 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 618BE156F3C for ; Mon, 5 Aug 2024 13:08:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863286; cv=none; b=kfHeWOGWZVmKEqe521gmOaDA/kCQSGiSQbqPJipwI4sZnwUHMle2JW/dCxXVAL+h+S/APoIC8s9GLsSTF9XbwPhqKi/DLlro20CuT06tB7cwaCoytoL+8ryVLfT0C0Bv5c22tjolmZD3Wl457it7Zqb0WNEPVi9QsaPWzGpcagQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863286; c=relaxed/simple; bh=YGXAcgYW1ixv2L36cvBAl5m2FgjnqyH9QEmD9DDOrIM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=p9VX1MJgOzFBg61Wa3RFcWbI8KgOWEwH6ClvKCbFScK2/ePTuAwkM3izkviFwBToxoyB2ZtU8YePtgIcrS2xeiXl/35EWBAiXCXl3Eb9G/wRMgOq3FJ5l3vlSUINtSO5/v+bWSH1FQ13fi5geKm7wFDacknRbqdajts9pO5b4js= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=g0LNwkgL; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=OrvaR4cH; arc=none smtp.client-ip=103.168.172.145 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="g0LNwkgL"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="OrvaR4cH" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfout.nyi.internal (Postfix) with ESMTP id B8016138FCB2; Mon, 5 Aug 2024 09:08:04 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Mon, 05 Aug 2024 09:08:04 -0400 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:subject :subject:to:to; s=fm3; t=1722863284; x=1722949684; bh=F/rE/KEYJN LjiZ9FugfZ1pGBz5Ho78FYTx3z1OlUvBw=; b=g0LNwkgLLUabVZZ+Zgfgr0T1sa CcgyKHjRHJKXnrKXcf0soYRtBtCBa6szs2k7vYw12mLoMVBUEkwZ15PTqgD14NRS +NRWQOrILtWYopUDohMJJXrVTMQ7XvClGLVbVpu7r02MNMJjrsilbeR+Nzi2EJte oj2OlW4DIcDUseHetnqR8t/a4zncqpK1zOu14o14H7j2kVamOu8MBkR5bwrMTdHZ 2Qyl2xIrTGyfF7JgzOibz8euXXUA2LFqgRQa6u6VrrZ0wiSAL8WhwOOMmRfizSe7 xBR2v6RsHhPq8VdOjYTh+J+nOsKiqqkgknLg5h+3wKx3k4IeyzYC9EezTYog== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863284; x=1722949684; bh=F/rE/KEYJNLjiZ9FugfZ1pGBz5Ho 78FYTx3z1OlUvBw=; b=OrvaR4cH6/HP/n30QSRLOdL+LSslC132kmXYe25CK7tg 4LTztSjx+Ff0NV+s60tO8mt90oXRbpe7XucAYIJX7FJydGkpKXyoybockgaszodC AkHOrLGqJo6iUnS8vRgKLk6kb9UE5kupMjPWCIGLI0NmSSVkk/bi4F5G0omvIGN6 +ANtxxXKW9hv9dV0SR8UTxFYMckm5hPlzcVULlLoUVDSRTnKOuThEkaY7DKK/v8w fQZo7NxhZTXnr5b4uUsmog51VRTuSQePyV0qB/kCzv6MvvKel8twVlY+eEuicznq vcVeyISo6uyibhoox3eVuJsa/rGeXktCFjrec6JgSg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:03 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 0f108191 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:02 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:01 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 4/9] reftable/stack: update stats on failed full compaction Message-ID: <1989dafcb468c3f6c84f802bee5016a8d22c4556.1722862822.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 auto-compaction fails due to a locking error, we update the statistics to indicate this failure. We're not doing the same when performing a full compaction. Fix this inconsistency by using `stack_compact_range_stats()`, which handles the stat update for us. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 14 +++++++------- reftable/stack_test.c | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index ba8234b486..e5959d2c76 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1205,13 +1205,6 @@ static int stack_compact_range(struct reftable_stack *st, return err; } -int reftable_stack_compact_all(struct reftable_stack *st, - struct reftable_log_expiry_config *config) -{ - return stack_compact_range(st, 0, st->merged->stack_len ? - st->merged->stack_len - 1 : 0, config); -} - static int stack_compact_range_stats(struct reftable_stack *st, size_t first, size_t last, struct reftable_log_expiry_config *config) @@ -1222,6 +1215,13 @@ static int stack_compact_range_stats(struct reftable_stack *st, return err; } +int reftable_stack_compact_all(struct reftable_stack *st, + struct reftable_log_expiry_config *config) +{ + size_t last = st->merged->stack_len ? st->merged->stack_len - 1 : 0; + return stack_compact_range_stats(st, 0, last, config); +} + static int segment_size(struct segment *s) { return s->end - s->start; diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 46d376026b..2b1eb83934 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1001,8 +1001,7 @@ static void test_reftable_stack_compaction_with_locked_tables(void) */ err = reftable_stack_compact_all(st, NULL); EXPECT(err == REFTABLE_LOCK_ERROR); - /* TODO: this is wrong, we should get notified about the failure. */ - EXPECT(st->stats.failures == 0); + EXPECT(st->stats.failures == 1); EXPECT(st->merged->stack_len == 3); reftable_stack_destroy(st); From patchwork Mon Aug 5 13:08:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753607 Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A2F85156F3C for ; Mon, 5 Aug 2024 13:08:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863292; cv=none; b=Qlh+dcNkIxmUsCPnxD29/TD9/4codXO+n96v1mQPnsxosOmVxkl2gcJC5OcQv17RoCwa69xzTSGjCWUFSjKIZnRmq+jL14xe4ogbeV1/TacqOj2X8/dxyNyhVhGtUb+P6O2pcZ8Z356Un9ONo6gdyDeQSsjJkbdDsHyTAo/NPwk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863292; c=relaxed/simple; bh=cijCqyF7TPHslqc8hPTiK61SpL7F8WrswfpkDPxKUoM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bldg0oNcc0Z3s8PK3RE+TPqw1rFYqDH2U+kNEPHZabGZsWx+SoeMHOFtiN7zGLcMcY/qM47yeEWkU+/OAkW2eTaBh3m9Rd45soJibmM0rcKd1alaURXh58YFtw9ViCtn+gWZ5iFa4hjNI+GAWbxZCap3fOLlZLC3DufHZQsGBhI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=MFfMUWcO; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=GFSQ/JVo; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="MFfMUWcO"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="GFSQ/JVo" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id D85A91151B53; Mon, 5 Aug 2024 09:08:09 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Mon, 05 Aug 2024 09:08:09 -0400 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:subject :subject:to:to; s=fm3; t=1722863289; x=1722949689; bh=/e00MIv4lk nCw+yX09KbKEVXBeUqzkVeJ0OORjPxiTU=; b=MFfMUWcOK4MaOjxS2ge7Q6fMRD T9Y1Egx/20wd0U6qYcyYGF68RHu07Byr00x9KxeseHCn1cJXCs2dXOqOes/dFd2M 2JBOGCteUopOrx1kJ64wPxHDYoQSMB9NScHzOM7sRyK7ZndZE6aUi49H/fYH3Jiu pLPalskmfQcunVDaUBMOkCBajFPbbns1bMxBtQeU3n4+0PJpOKMMeJE5ZiEi9uKU B/ht8iPyNibNLzix/jm719bK0pi8dUHuvBtRWtc50cZ9TQgt/sRUjmKQh6IUHSoW q+8EVwvnpsPmot6/RPtLrKfY6bFmzRPJt23RwhmawTiSbBTt2fr/edpang/g== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863289; x=1722949689; bh=/e00MIv4lknCw+yX09KbKEVXBeUq zkVeJ0OORjPxiTU=; b=GFSQ/JVolXfx6KJDoopI3Bcx5/opOHYX7UvMnFtsHfxs UrOAamhNNIhpzHNFspBnV5vBzem9ASsR4+ku7GJ5VP+wKP4x67nwU1NyWZGzZBXk ACGEeXhNaJfQU+wHIReT2j0W12IL+ldK0eSqyxPelR+bqT5X2Xuexl8kMF8e3lyw pQGOMJ/mk1EBUOBdiDmZa2NhKHHPXWPvuUs1RcJDCQhxHWC5XN3i74x1bAtesptC CVoJYeWwJolUd1NokebNSSEsiFizilJ8h8tf4IrOFWV5v5znBq029D4FKn0BSs7m dd5WD9LCq5sV+dytIkpYzfcEdX8XkRYskm2kxvFvXw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:08 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c7ba2821 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:07 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:06 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 5/9] reftable/stack: simplify tracking of table locks Message-ID: <73e5d104eb96d4b1894ee3fb1ddf7be7e187ec91.1722862822.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 compacting tables, we store the locks of all tables we are about to compact in the `table_locks` array. As we currently only ever compact all tables in the user-provided range or none, we simply track those locks via the indices of the respective tables in the merged stack. This is about to change though, as we will introduce a mode where auto compaction gracefully handles the case of already-locked files. In this case, it may happen that we only compact a subset of the user-supplied range of tables. In this case, the indices will not necessarily match the lock indices anymore. Refactor the code such that we track the number of locks via a separate variable. The resulting code is expected to perform the same, but will make it easier to perform the described change. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index e5959d2c76..07e7ffc6b9 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1016,7 +1016,7 @@ static int stack_compact_range(struct reftable_stack *st, struct lock_file *table_locks = NULL; struct tempfile *new_table = NULL; int is_empty_table = 0, err = 0; - size_t i; + size_t i, nlocks = 0; if (first > last || (!expiry && first == last)) { err = 0; @@ -1051,7 +1051,7 @@ static int stack_compact_range(struct reftable_stack *st, for (i = first; i <= last; i++) { stack_filename(&table_name, st, reader_name(st->readers[i])); - err = hold_lock_file_for_update(&table_locks[i - first], + err = hold_lock_file_for_update(&table_locks[nlocks], table_name.buf, LOCK_NO_DEREF); if (err < 0) { if (errno == EEXIST) @@ -1066,7 +1066,7 @@ static int stack_compact_range(struct reftable_stack *st, * run into file descriptor exhaustion when we compress a lot * of tables. */ - err = close_lock_file_gently(&table_locks[i - first]); + err = close_lock_file_gently(&table_locks[nlocks++]); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; @@ -1183,8 +1183,8 @@ static int stack_compact_range(struct reftable_stack *st, * Delete the old tables. They may still be in use by concurrent * readers, so it is expected that unlinking tables may fail. */ - for (i = first; i <= last; i++) { - struct lock_file *table_lock = &table_locks[i - first]; + for (i = 0; i < nlocks; i++) { + struct lock_file *table_lock = &table_locks[i]; char *table_path = get_locked_file_path(table_lock); unlink(table_path); free(table_path); @@ -1192,8 +1192,8 @@ static int stack_compact_range(struct reftable_stack *st, done: rollback_lock_file(&tables_list_lock); - for (i = first; table_locks && i <= last; i++) - rollback_lock_file(&table_locks[i - first]); + for (i = 0; table_locks && i < nlocks; i++) + rollback_lock_file(&table_locks[i]); reftable_free(table_locks); delete_tempfile(&new_table); From patchwork Mon Aug 5 13:08:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753608 Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BA48B156F3C for ; Mon, 5 Aug 2024 13:08:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863296; cv=none; b=BRzcagvsV3moDDaAJhT4tMigb3Zrg5JZD4rwiEtmYRr+vvmQOb2uaIvUG3sr7h+IwtxpAMa3L3lFS3ujw8wyJjYvc8NYSjSMm8rYJUf4t5fUc2k3J/Ujsedyol0YaNTiG5dDHVxyuZKdXC6HVUa+xl+KGFu6ofvIZk3JlD2NIjg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863296; c=relaxed/simple; bh=HmVi895vZT8rqBuueQd42lp5zDwDvvhpqsz8c7UCfMw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=XuxnGwRah9Mc7NMxcKYNBGY+H3MbhdQvnRsgmVUyY8WLaBJom2ZzA6A6DibcI09bMf1MrhbBlQLRFzvK5JjBbXFx+78FqTBERpLdNhX3Yf3YmAP06eSwwSVrYV7VfKoSrjbjBlyXqlKCKVFqXnxLuyPuTrYVO6/nMyvlhyMn1hk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=BGfoH0Vx; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ch0AzQ7W; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="BGfoH0Vx"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ch0AzQ7W" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 0DFB01151B4D; Mon, 5 Aug 2024 09:08:14 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 05 Aug 2024 09:08:14 -0400 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:subject :subject:to:to; s=fm3; t=1722863294; x=1722949694; bh=MByDtItTSH DNYOCjK/fsP0Dana783yODbOKgrg+J16g=; b=BGfoH0VxY+E8qfE3yciAtowaeP lzw1nVb0xfaj7vHW8BGuK3tnlkdbCRgQRNLHxDGITHJdK6SvK5Ek8/J607YjvnVH qFOXdTUFcbU2zq1t/LKNN4KiwUPPRzMYC6qBgl2BQJo4+HO450kvcgbCf8mMuPYx SBr/uTaF4wz4tQCmd/qMa2iDiuMccsPDCc3GJTI2yLWFTLPjCxq6p+HovE/LLZzZ E5VRVj08Bk0Pu/12IQYFpkkFdvIo9ef87VeAZvCsi9jfYXBRaVJl6UteLw4CKq8W HGu0W/0p2CfHGvkQE2xwxBWmcXstwH4jAcV2RZjhKb2vOZbQngPCDh2Fq7nw== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863294; x=1722949694; bh=MByDtItTSHDNYOCjK/fsP0Dana78 3yODbOKgrg+J16g=; b=ch0AzQ7WlE+jukD9x5XJ1poyDU6TLo3bx4AjKMFYE5Ii 22m0ug2GXzCFWFwZXlOp7fcPeAqyjYcJcVLgRNqHm7jncNUYE4NHdvfnRVqBnmFN jF2ZGJQ0z+4IHgAaWQPt13+ArwzNsm23zc4bWaIg6h5ga7vh+YiQZUQ0dwXk7R05 Xxg9b+NBytrmuPy4aKOwCPJhFXtrFGKKJcbZw2eNHY2/B6VJZd1xKC+iRcbl+V9s 0bsyf2IT16Yfw9Aq+7kFneU0ReUfRd4LkyzWgY5h05gQbgUB7GGj1pU/WUCH6U/r V1EtK6K+Lj5FhDzbsRzYioeFk+hm1o4tFLN3pzxvWw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:13 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id f1550fba (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:12 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:10 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 6/9] reftable/stack: do not die when fsyncing lock file files 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: We use `fsync_component_or_die()` when committing an addition to the "tables.list" lock file, which unsurprisingly dies in case the fsync fails. Given that this is part of the reftable library, we should never die and instead let callers handle the error. Adapt accordingly and use `fsync_component()` instead. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 07e7ffc6b9..9ca549294f 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -674,8 +674,11 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - fsync_component_or_die(FSYNC_COMPONENT_REFERENCE, lock_file_fd, - get_tempfile_path(add->lock_file)); + err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd); + if (err < 0) { + err = REFTABLE_IO_ERROR; + goto done; + } err = rename_tempfile(&add->lock_file, add->stack->list_file); if (err < 0) { From patchwork Mon Aug 5 13:08:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753609 Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0C1AE159583 for ; Mon, 5 Aug 2024 13:08:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863301; cv=none; b=DpGmIiVtMp55dv2zT368Fm4NT3kHBHfCPpggTGfuMF3FDwVKa/uM8uK1aB+XuOyXFAxwGdpPMXGA7p7khsWhtwtpfbVtmDpDo1eCGIU9iyd6lal3kFzh8tgeuWmPycBUxVbEHlLBG9Qb2hQ5OjODdIqRSGfG8/VaRqaQ0aIXMek= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863301; c=relaxed/simple; bh=KeLqwWHj4pG4067Ud5Q4dyFuzcqpWvYVS0ni1Ys0lR0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Lu00tR1f+iCnUT0AdvJgPivR8SN+5fwPguGODBqJroej3YS7zIgoKA/ce/tSY0B25lnU7yXHQ17niZKGuKVYYz3n03AWx8cjK9/N/qNIFax060jmw2MZikHvMVBy1n5Zg4GYPzuyg3f2wwLrhhwxnjlw8uALpIeHvOcN+R/ks1g= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=mGe8pEj5; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=UIMwI9JC; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="mGe8pEj5"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="UIMwI9JC" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 257F11151B75; Mon, 5 Aug 2024 09:08:19 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 05 Aug 2024 09:08:19 -0400 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:subject :subject:to:to; s=fm3; t=1722863299; x=1722949699; bh=YMcltKA7JO 5S703QCXbBcvKA9XODbzGQ5E9NzaK2Zlc=; b=mGe8pEj5ZiuqkTvP6InCWt4ESH Tw9OMia/hmYj6OMy05msqVyNq+P7TntXYZHkt575PpVo9DkKit8wOLrSd27UcReG 6EKgOuI04CCzarwRHNCILKa5XDliBHSxJ6GBNk/RIAM7QP1ezrJouDNiCjAE0YaI Hep2oP/S0CNC73sziIjPqWL2+x1ge7vvShkIg+Blk4Wt4a9+poV5Z1Wb7Pq1Q5PW KlCKEDYkrFnvfB8CoLPf1Du1McYzL8bsV/GyosqQfq4fRtKDXZLQeEwZYBNs3ids GManPkDp17kkbdwfDDiFX/+t0Uo4vzxAb48oC3eIydKq3foQgn6kvUlK6sJA== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863299; x=1722949699; bh=YMcltKA7JO5S703QCXbBcvKA9XOD bzGQ5E9NzaK2Zlc=; b=UIMwI9JCyTqEsCbio1YbyVTmQudRbLlIcyXr5VqvmT4F Pxv65CM2PX+mx/VfC24EFYDktQetqT6tuTgnL8j+6jvgCOh49K5lSHH3ycQFeRu0 pQWcP+l9lBDa+k5ej9fjSYALW4CB4kcvhtU/x8mAG9cjmPokYoJaZOgyR/RuOukl HJ+HycqTxST175iRfRcew+0Uw2p4bGY63nhceVT8hgFtk0bm75n0a2JBxVZiIqws 2h6Qa9HoAX1PDCKbKAqF5jRy7sqelhiNcjBjtr5a2cRp1Pr4l91wVP374zyJkwo2 leGJAIZByqwTMOXffdpEgsrl83A85u5CzKaz+8UCZg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:18 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id b8b8d3ba (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:17 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:15 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 7/9] reftable/stack: use lock_file when adding table to "tables.list" 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 modifying "tables.list", we need to lock the list before updating it to ensure that no concurrent writers modify the list at the same point in time. While we do this via the `lock_file` subsystem when compacting the stack, we manually handle the lock when adding a new table to it. While not wrong, it is at least inconsistent. Refactor the code to consistently lock "tables.list" via the `lock_file` subsytem. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 9ca549294f..54982e0f7d 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -567,7 +567,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max) } struct reftable_addition { - struct tempfile *lock_file; + struct lock_file tables_list_lock; struct reftable_stack *stack; char **new_tables; @@ -581,13 +581,13 @@ 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; + int err; - strbuf_addf(&lock_file_name, "%s.lock", st->list_file); + add->stack = st; - add->lock_file = create_tempfile(lock_file_name.buf); - if (!add->lock_file) { + err = hold_lock_file_for_update(&add->tables_list_lock, st->list_file, + LOCK_NO_DEREF); + if (err < 0) { if (errno == EEXIST) { err = REFTABLE_LOCK_ERROR; } else { @@ -596,7 +596,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; } if (st->opts.default_permissions) { - if (chmod(add->lock_file->filename.buf, st->opts.default_permissions) < 0) { + if (chmod(get_lock_file_path(&add->tables_list_lock), + st->opts.default_permissions) < 0) { err = REFTABLE_IO_ERROR; goto done; } @@ -635,7 +636,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables_len = 0; add->new_tables_cap = 0; - delete_tempfile(&add->lock_file); + rollback_lock_file(&add->tables_list_lock); strbuf_release(&nm); } @@ -651,7 +652,7 @@ 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 lock_file_fd = get_lock_file_fd(&add->tables_list_lock); int err = 0; size_t i; @@ -680,7 +681,7 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err = rename_tempfile(&add->lock_file, add->stack->list_file); + err = commit_lock_file(&add->tables_list_lock); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; From patchwork Mon Aug 5 13:08:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753610 Received: from fhigh7-smtp.messagingengine.com (fhigh7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DFE9F15574D for ; Mon, 5 Aug 2024 13:08:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863305; cv=none; b=W4gA4UGW6nLQiFGewjI6WMYSicrKMleeyWBjXKVc1SCgUpSF8xX9g6qRJHndv1EVvD/M5wG6qlLpzM71AWbC9XbwU+zQNoI/K8rbYpXKOMBNqyUvG2S1zVKknTZPMypKjuzj+gKmUzcsyp91mNkvvw8IzalOv8DCqGy9SubN8BQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863305; c=relaxed/simple; bh=MOALVfFqh8Au/t6BO864DbrpOJbCzHsP7/Sf06f1mSI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VnHVdXSSDKn3UqUaO3zPyFRvGQKd5nL1wsgake5RO56RZ6dLLqR/zthZl/eHabq2XsSqgaO5T+PX0BTDASsHJcC5H0pW1uzLicZE3XMA5+lmfviNDSnRSAfbfw59jiRlkai84xqljzo7OYoLTJjVhMSafwOB37IwT4yNmJ5TTMA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=EaipVMkf; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=R96aohp+; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="EaipVMkf"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="R96aohp+" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 424F41151B79; Mon, 5 Aug 2024 09:08:23 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Mon, 05 Aug 2024 09:08:23 -0400 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:subject :subject:to:to; s=fm3; t=1722863303; x=1722949703; bh=kxm/gXzt4Z gQQ56nRNKDvwwun8WWArxfwXkVhtOCBSs=; b=EaipVMkfmq//qP3xCtiLZNG+fm Uukr7umuYDcFIb1MkTCSduFgXC2aQ4LYOs04A2qdLx+X5mbS6QpBH1xNJHg1R/d4 KeNNf2wVqSFW0Tulso58KRfajG04mQkgbPDiboaHT5RXBaEuzBNAJTPBgB2EUeu0 H2MnxpCjmCFxFMuq0ONErKjktsqQKyWF3MqceypGm0Hi+VkMaKftw10aQ+NyRwzf A5MjHisuWM6Mleifkpq34IMCwhuV9JA7suOlZrbPS8dBcd5L2MJti+oYB3NWgwgZ awSiuhodX+ep/sfJ9FRkBjOqBhWtYUJfD7SfIeUHVT4alBr8GMNRvRKDVITA== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863303; x=1722949703; bh=kxm/gXzt4ZgQQ56nRNKDvwwun8WW ArxfwXkVhtOCBSs=; b=R96aohp+H95q7m97WYwi1Fbio69QtP2spMbyvyYJ+Oo8 KX2SogFEuQgE9VEvoY5oe5M5RJWLYqptrahc3IIEX3hrDNkIRSiHrnlbsB3WvKl8 G+IqAq+PXhEvRSkTrEdRbWNw0hQV6drMSaxA/czr3PBo74fzUcFoW+JUNmdILFx9 zCGZe1r7wmz1Oz9kS6Dek/U5F28xx6kRebO7v5OmulUFGyWA0VPSIEeJHWLl9vwU ZVDjh55x2xlWLQxvvHWDqxbiwvWcr5z33xswtkndPU74fsfxoj2gSCabRL4XfygO x6NQFiluUrUAV5bhnbPKe5M222NFagTvHUmjN/io+g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:22 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ba3e2f40 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:21 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:20 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 8/9] reftable/stack: fix corruption on concurrent 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: The locking employed by compaction uses the following schema: 1. Lock "tables.list" and verify that it matches the version we have loaded in core. 2. Lock each of the tables in the user-supplied range of tables that we are supposed to compact. These locks prohibit any concurrent process to compact those tables while we are doing that. 3. Unlock "tables.list". This enables concurrent processes to add new tables to the stack, but also allows them to compact tables outside of the range of tables that we have locked. 4. Perform the compaction. 5. Lock "tables.list" again. 6. Move the compacted table into place. 7. Write the new order of tables, including the compacted table, into the lockfile. 8. Commit the lockfile into place. Letting concurrent processes modify the "tables.list" file while we are doing the compaction is very much part of the design and thus expected. After all, it may take some time to compact tables in the case where we are compacting a lot of very large tables. But there is a bug in the code. Suppose we have two processes which are compacting two slices of the table. Given that we lock each of the tables before compacting them, we know that the slices must be disjunct from each other. But regardless of that, compaction performed by one process will always impact what the other process needs to write to the "tables.list" file. Right now, we do not check whether the "tables.list" has been changed after we have locked it for the second time in (5). This has the consequence that we will always commit the old, cached in-core tables to disk without paying to respect what the other process has written. This scenario would then lead to data loss and corruption. This can even happen in the simpler case of one compacting process and one writing process. The newly-appended table by the writing process would get discarded by the compacting process because it never sees the new table. Fix this bug by re-checking whether our stack is still up to date after locking for the second time. If it isn't, then we adjust the indices of tables to replace in the updated stack. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 107 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 102 insertions(+), 5 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 54982e0f7d..51eb4470c7 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1020,7 +1020,9 @@ static int stack_compact_range(struct reftable_stack *st, struct lock_file *table_locks = NULL; struct tempfile *new_table = NULL; int is_empty_table = 0, err = 0; + size_t first_to_replace, last_to_replace; size_t i, nlocks = 0; + char **names = NULL; if (first > last || (!expiry && first == last)) { err = 0; @@ -1123,6 +1125,100 @@ static int stack_compact_range(struct reftable_stack *st, } } + /* + * As we have unlocked the stack while compacting our slice of tables + * it may have happened that a concurrently running process has updated + * the stack while we were compacting. In that case, we need to check + * whether the tables that we have just compacted still exist in the + * stack in the exact same order as we have compacted them. + * + * If they do exist, then it is fine to continue and replace those + * tables with our compacted version. If they don't, then we need to + * abort. + */ + err = stack_uptodate(st); + if (err < 0) + goto done; + if (err > 0) { + ssize_t new_offset = -1; + int fd; + + fd = open(st->list_file, O_RDONLY); + if (fd < 0) { + err = REFTABLE_IO_ERROR; + goto done; + } + + err = fd_read_lines(fd, &names); + close(fd); + if (err < 0) + goto done; + + /* + * Search for the offset of the first table that we have + * compacted in the updated "tables.list" file. + */ + for (size_t i = 0; names[i]; i++) { + if (strcmp(names[i], st->readers[first]->name)) + continue; + + /* + * We have found the first entry. Verify that all the + * subsequent tables we have compacted still exist in + * the modified stack in the exact same order as we + * have compacted them. + */ + for (size_t j = 1; j < last - first + 1; j++) { + const char *old = first + j < st->merged->stack_len ? + st->readers[first + j]->name : NULL; + const char *new = names[i + j]; + + /* + * If some entries are missing or in case the tables + * have changed then we need to bail out. Again, this + * shouldn't ever happen because we have locked the + * tables we are compacting. + */ + if (!old || !new || strcmp(old, new)) { + err = REFTABLE_OUTDATED_ERROR; + goto done; + } + } + + new_offset = i; + break; + } + + /* + * In case we didn't find our compacted tables in the stack we + * need to bail out. In theory, this should have never happened + * because we locked the tables we are compacting. + */ + if (new_offset < 0) { + err = REFTABLE_OUTDATED_ERROR; + goto done; + } + + /* + * We have found the new range that we want to replace, so + * let's update the range of tables that we want to replace. + */ + last_to_replace = last + (new_offset - first); + first_to_replace = new_offset; + } else { + /* + * `fd_read_lines()` uses a `NULL` sentinel to indicate that + * the array is at its end. As we use `free_names()` to free + * the array, we need to include this sentinel value here and + * thus have to allocate `stack_len + 1` many entries. + */ + REFTABLE_CALLOC_ARRAY(names, st->merged->stack_len + 1); + for (size_t i = 0; i < st->merged->stack_len; i++) + names[i] = xstrdup(st->readers[i]->name); + last_to_replace = last; + first_to_replace = first; + } + /* * If the resulting compacted table is not empty, then we need to move * it into place now. @@ -1145,12 +1241,12 @@ static int stack_compact_range(struct reftable_stack *st, * have just written. In case the compacted table became empty we * simply skip writing it. */ - for (i = 0; i < first; i++) - strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name); + for (i = 0; i < first_to_replace; i++) + strbuf_addf(&tables_list_buf, "%s\n", names[i]); if (!is_empty_table) strbuf_addf(&tables_list_buf, "%s\n", new_table_name.buf); - for (i = last + 1; i < st->merged->stack_len; i++) - strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name); + for (i = last_to_replace + 1; names[i]; i++) + strbuf_addf(&tables_list_buf, "%s\n", names[i]); err = write_in_full(get_lock_file_fd(&tables_list_lock), tables_list_buf.buf, tables_list_buf.len); @@ -1203,9 +1299,10 @@ static int stack_compact_range(struct reftable_stack *st, delete_tempfile(&new_table); strbuf_release(&new_table_name); strbuf_release(&new_table_path); - strbuf_release(&tables_list_buf); strbuf_release(&table_name); + free_names(names); + return err; } From patchwork Mon Aug 5 13:08:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13753611 Received: from fout2-smtp.messagingengine.com (fout2-smtp.messagingengine.com [103.168.172.145]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5126D18026 for ; Mon, 5 Aug 2024 13:08:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.145 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863311; cv=none; b=olKoy7a3luQ8M46I/RXnesAZzAUc0xwBQQZfQjUVHmYyzIuNdXR3Pd1Df2wL0aEiHl6ZM8Gc353LT8y6IUB9MNaubqXLN+nYVE4nLo2mYiTVZXeem4WM1lSh82rE5F2lm8LlO7pmOOY2iIrKTJKVMebUtLl1CpdZLw7eXUIeLC4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722863311; c=relaxed/simple; bh=mXal+VMIj7NC68//vH4yjzgdjni8omilFbJEyIOz2f0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=m+kwAOFGU0phVJbKV94iCUuusX6fuHo6kLTRvq5PMY61ZzgIhNoxxIl+BT9ykbCCWmZ+QEzJWzLNgdfCFPzgpeZMJcSo/2r2+n/CTaE0QFAa7AaeV4ks6L0RXr8PDmBnM39HWHDW8kmxm8OIownsvFhZd6tcKwVi5kVwvn41m9k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=jfU8BMIR; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Ty8aS8XY; arc=none smtp.client-ip=103.168.172.145 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="jfU8BMIR"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Ty8aS8XY" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfout.nyi.internal (Postfix) with ESMTP id 66500138FCB6; Mon, 5 Aug 2024 09:08:28 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Mon, 05 Aug 2024 09:08:28 -0400 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:subject :subject:to:to; s=fm3; t=1722863308; x=1722949708; bh=8L2p298u5x b8mM5jVHzXOG7F0hZZdn91TanCL6cF29w=; b=jfU8BMIR4qaxVR5vLnP/q296qc mnsk4y75I4PmSgXvkTyc+DC4Sq0DOnXgnDfWv/r72BU8bYwcmDaBq6099MSNO7iA LX78o7bISgpnIfOwN8Zy/KAV4h6WCEepxm7G178SSqLC9GfM5HMgyzoz1eRCVyGC 02ZLN/234vYOK1m1+ZuCOyslx+TJTFgauyUd5Iji2hHqw2Ch9GA1WRzbs8047r8I +5uac8SfHnzWSC+gnn8dqS2LcAPCZ+EhioeUOXWljqvP3lsG5duhbAxW5mtKrhUW 5riONZL+Iwaxb1WIe1kta/JkAUvzkdY6iFVQdXyMBXqwV2tANCcaRxDNNCBA== 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:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1722863308; x=1722949708; bh=8L2p298u5xb8mM5jVHzXOG7F0hZZ dn91TanCL6cF29w=; b=Ty8aS8XYWJeQxk0N/OdNy1fET4LHuCa3lgtZyObH9B3U ZnDTydWJs9z9wLxQzK//cy5pVHvWSaNiG/lJ/RvXLe33gES98dwgD5JThhP08GIV aq7eZysvrcdZKtsoqO4eUhwpfRBz5PvCw11vs8evHnpepktTf22CeWboadQ//Ry4 3JK4cRbi7Ujp0wE2zMQs9c8h1P3mSOMAXhxhCA746Ww+rlxhqvXBzdHyMUpxR120 rBSVLDonos7lUjtwMM5U8s5oZxrK88c1ppF+IviqUu0awr0iVxavXS0L8AQ0BZBR XlubKpx21pNitDQzstsnKd6B+WwMJyJfcpPKynNgog== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrkeeigdeiudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeehgfeltddtheejleffteen ucevlhhushhtvghrufhiiigvpeefnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 5 Aug 2024 09:08:27 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 03cc79f4 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 5 Aug 2024 13:08:26 +0000 (UTC) Date: Mon, 5 Aug 2024 15:08:25 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano Subject: [PATCH v2 9/9] reftable/stack: handle locked tables during auto-compaction Message-ID: <1ef560feb1906c160ad7c81a30e9bd4fc92d2eca.1722862822.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 compacting tables, it may happen that we want to compact a set of tables which are already locked by a concurrent process that compacts them. In the case where we wanted to perform a full compaction of all tables it is sensible to bail out in this case, as we cannot fulfill the requested action. But when performing auto-compaction it isn't necessarily in our best interest of us to abort the whole operation. For example, due to the geometric compacting schema that we use, it may be that process A takes a lot of time to compact the bulk of all tables whereas process B appends a bunch of new tables to the stack. B would in this case also notice that it has to compact the tables that process A is compacting already and thus also try to compact the same range, probably including the new tables it has appended. But because those tables are locked already, it will fail and thus abort the complete auto-compaction. The consequence is that the stack will grow longer and longer while A isn't yet done with compaction, which will lead to a growing performance impact. Instead of aborting auto-compaction altogether, let's gracefully handle this situation by instead compacting tables which aren't locked. To do so, instead of locking from the beginning of the slice-to-be-compacted, we start locking tables from the end of the slice. Once we hit the first table that is locked already, we abort. If we succeded to lock two or more tables, then we simply reduce the slice of tables that we're about to compact to those which we managed to lock. This ensures that we can at least make some progress for compaction in said scenario. It also helps in other scenarios, like for example when a process died and left a stale lockfile behind. In such a case we can at least ensure some compaction on a best-effort basis. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 59 +++++++++++++++++++++++++++++++------- reftable/stack_test.c | 12 ++++---- t/t0610-reftable-basics.sh | 21 +++++++++----- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 51eb4470c7..b0721640e4 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -999,6 +999,15 @@ static int stack_write_compact(struct reftable_stack *st, return err; } +enum stack_compact_range_flags { + /* + * Perform a best-effort compaction. That is, even if we cannot lock + * all tables in the specified range, we will try to compact the + * remaining slice. + */ + STACK_COMPACT_RANGE_BEST_EFFORT = (1 << 0), +}; + /* * Compact all tables in the range `[first, last)` into a single new table. * @@ -1010,7 +1019,8 @@ static int stack_write_compact(struct reftable_stack *st, */ static int stack_compact_range(struct reftable_stack *st, size_t first, size_t last, - struct reftable_log_expiry_config *expiry) + struct reftable_log_expiry_config *expiry, + unsigned int flags) { struct strbuf tables_list_buf = STRBUF_INIT; struct strbuf new_table_name = STRBUF_INIT; @@ -1052,19 +1062,47 @@ static int stack_compact_range(struct reftable_stack *st, /* * Lock all tables in the user-provided range. This is the slice of our * stack which we'll compact. + * + * Note that we lock tables in reverse order from last to first. The + * intent behind this is to allow a newer process to perform best + * effort compaction of tables that it has added in the case where an + * older process is still busy compacting tables which are preexisting + * from the point of view of the newer process. */ REFTABLE_CALLOC_ARRAY(table_locks, last - first + 1); - for (i = first; i <= last; i++) { - stack_filename(&table_name, st, reader_name(st->readers[i])); + for (i = last + 1; i > first; i--) { + stack_filename(&table_name, st, reader_name(st->readers[i - 1])); err = hold_lock_file_for_update(&table_locks[nlocks], table_name.buf, LOCK_NO_DEREF); if (err < 0) { - if (errno == EEXIST) + /* + * When the table is locked already we may do a + * best-effort compaction and compact only the tables + * that we have managed to lock so far. This of course + * requires that we have been able to lock at least two + * tables, otherwise there would be nothing to compact. + * In that case, we return a lock error to our caller. + */ + if (errno == EEXIST && last - (i - 1) >= 2 && + flags & STACK_COMPACT_RANGE_BEST_EFFORT) { + err = 0; + /* + * The subtraction is to offset the index, the + * addition is to only compact up to the table + * of the preceding iteration. They obviously + * cancel each other out, but that may be + * non-obvious when it was omitted. + */ + first = (i - 1) + 1; + break; + } else if (errno == EEXIST) { err = REFTABLE_LOCK_ERROR; - else + goto done; + } else { err = REFTABLE_IO_ERROR; - goto done; + goto done; + } } /* @@ -1308,9 +1346,10 @@ static int stack_compact_range(struct reftable_stack *st, static int stack_compact_range_stats(struct reftable_stack *st, size_t first, size_t last, - struct reftable_log_expiry_config *config) + struct reftable_log_expiry_config *config, + unsigned int flags) { - int err = stack_compact_range(st, first, last, config); + int err = stack_compact_range(st, first, last, config, flags); if (err == REFTABLE_LOCK_ERROR) st->stats.failures++; return err; @@ -1320,7 +1359,7 @@ int reftable_stack_compact_all(struct reftable_stack *st, struct reftable_log_expiry_config *config) { size_t last = st->merged->stack_len ? st->merged->stack_len - 1 : 0; - return stack_compact_range_stats(st, 0, last, config); + return stack_compact_range_stats(st, 0, last, config, 0); } static int segment_size(struct segment *s) @@ -1427,7 +1466,7 @@ int reftable_stack_auto_compact(struct reftable_stack *st) reftable_free(sizes); if (segment_size(&seg) > 0) return stack_compact_range_stats(st, seg.start, seg.end - 1, - NULL); + NULL, STACK_COMPACT_RANGE_BEST_EFFORT); return 0; } diff --git a/reftable/stack_test.c b/reftable/stack_test.c index 2b1eb83934..733cf6113e 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -913,13 +913,15 @@ static void test_reftable_stack_auto_compaction_with_locked_tables(void) write_file_buf(buf.buf, "", 0); /* - * Ideally, we'd handle the situation where any of the tables is locked - * gracefully. We don't (yet) do this though and thus fail. + * When parts of the stack are locked, then auto-compaction does a best + * effort compaction of those tables which aren't locked. So while this + * would in theory compact all tables, due to the preexisting lock we + * only compact the newest two tables. */ err = reftable_stack_auto_compact(st); - EXPECT(err == REFTABLE_LOCK_ERROR); - EXPECT(st->stats.failures == 1); - EXPECT(st->merged->stack_len == 5); + EXPECT_ERR(err); + EXPECT(st->stats.failures == 0); + EXPECT(st->merged->stack_len == 4); reftable_stack_destroy(st); strbuf_release(&buf); diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index b06c46999d..37510c2b2a 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -478,19 +478,26 @@ test_expect_success "$command: auto compaction" ' test_oid blob17_2 | git hash-object -w --stdin && - # Lock all tables write some refs. Auto-compaction will be - # unable to compact tables and thus fails gracefully, leaving - # the stack in a sub-optimal state. - ls .git/reftable/*.ref | + # Lock all tables, write some refs. Auto-compaction will be + # unable to compact tables and thus fails gracefully, + # compacting only those tables which are not locked. + ls .git/reftable/*.ref | sort | while read table do - touch "$table.lock" || exit 1 + touch "$table.lock" && + basename "$table" >>tables.expect || exit 1 done && + test_line_count = 2 .git/reftable/tables.list && git branch B && git branch C && - rm .git/reftable/*.lock && - test_line_count = 4 .git/reftable/tables.list && + # The new tables are auto-compacted, but the locked tables are + # left intact. + test_line_count = 3 .git/reftable/tables.list && + head -n 2 .git/reftable/tables.list >tables.head && + test_cmp tables.expect tables.head && + + rm .git/reftable/*.lock && git $command --auto && test_line_count = 1 .git/reftable/tables.list )