From patchwork Wed Jul 31 14:14:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13748832 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 97C931B373C for ; Wed, 31 Jul 2024 14:14:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435299; cv=none; b=VOeHVctMIRfEm0UvymrARh22C2WtvL6/9K/Ngu2kytAmGvj0d/17S8lu2X9hrjJSIbLxXp8v9sTjhTnyWhFmmC53Ndk4cXz1Re3MF7jaeJY9UefwrrrdqUx3+DbFCSyklD2pjs0yO7YatIjev4n+2oJygCn7nj/y2OFP8i5HjdI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435299; c=relaxed/simple; bh=YQX8HG6eLH4jQmBe+dnf7jOy3WIuyzRmlc6qxJn6e/s=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=T5ySlJudxAjcIOxGgWr2LNfd65c8JBeAiEhxd6+FQC6W7oVSRVuIDVf9s2zHRFB1lWzC+tay+aSQWH3D40AOJN/nnoghXXCyyh7Iv1wohs/q3kbEUbfL/cm3UFlglxmsNaRFoBAIO+DN4daPXHdBgm3g1Z2O1WZJp4+kyIdUSnk= 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=foswGz8u; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=nyXUqc0C; arc=none smtp.client-ip=103.168.172.156 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="foswGz8u"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="nyXUqc0C" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 3FA2211405BB for ; Wed, 31 Jul 2024 10:14:56 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Wed, 31 Jul 2024 10:14:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435296; x=1722521696; bh=4M7FHH6ltT 7+ZHSCbp21PRyg4mjAmIlgQr0pLWvpOUc=; b=foswGz8uhpgEfp0vAAmuSaDpsv FNnDztVr0RdoIlq9r8jUlIk6hbiWi+uIpAQwXvk9+DflWdobyOJDTxPfzNiZqu31 z4sB9YkNPe2570jEENeIds6lCzsQ2ISUI7S9SQbyMwHpsUr0BNKKTOBFZ87F8vQn ok/3KjK2eWHrqB1foQfvJwZBZdgM+5gS8rCeqzHsl/0yuDCo5xDgm1dGlPwWjWlB 2DkY3RVOlgWr8r6X7KvmpuC2U0I2lk/BV3/umvOzJv3PQCUcFed4gX6P/8q7OfFf 54iOAhCx0T1CFPRhbjye5WLan7LFAk1kbVHpUBrlGwyKsK1D2pH5L4XZTglg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435296; x=1722521696; bh=4M7FHH6ltT7+ZHSCbp21PRyg4mjA mIlgQr0pLWvpOUc=; b=nyXUqc0CPu8PU5Bqp1fXmW+EYL7ZXzJzMWQ2AsXvzWwE qZS7MODlYxLlMeCg+ywxDsIybmbP5qTb89eLESK2DN9Fo2G4owZkzhpRoBwqUkQC R9iucuHgNWF6B+c7R0KdO3EZUaKqD/957+9gYa6AL21Cbe49zLAhFtpnorWhlhSX 20iXMPvmIBaRIfZLe+6WaC9eGOLnE94URj/gSkwrIm1jmQd1c4vs+/EcSq+qKPDR DCTJELiYTiIv8X1u54bHitEfrpl1zUwrhQUKZQRkQyefsGrVdVajYorRVu5BNOhH v0VDm5yBWSNyr5ecui6ULw3rGUj9fcX+HvsnImFJzw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:14:55 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id fbd72f7e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:25 +0000 (UTC) Date: Wed, 31 Jul 2024 16:14:52 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 1/8] reftable/stack: refactor function to gather table sizes Message-ID: <5d99191f5c30927f01f9281dcccfa51a120fc698.1722435214.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 Wed Jul 31 14:14:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13748833 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (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 B8C0B1B375C for ; Wed, 31 Jul 2024 14:15:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435302; cv=none; b=LVV4tcy2VOkTtWf/K84sXx4g0DkUnPpt44Iu060FbJzNNd+GSu1qlPofykgdxz+JYNOyZSELkjxfAaPSAWS1UH321WWMkGvy7EJk3XAeI7CBW4nrwnlYr0LACHYq47rTiOkjlILJVG3RBRVzk7ril177+Lght4SkeOaq8l2Rkv4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435302; c=relaxed/simple; bh=ygkhdS004+btHxYp9hjaisI/UZJm7p/hK9WjtY/NvZk=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=UqE1aOza1H/2v18IjZKFcymMN8YQNyKRITY2UTe4DwcKU9IAb9FxSppjTcso8gjzpNhxq5pIB0Ym933k4FcgNgHYDrIhjZJahUZeQm3/f1xKLA1rVK0oVAJayHUxgnQZ5yHXt2Bw5nPYkr9p3FVFqxW96gfo0K9qXA7Uzg9WWjc= 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=IJjHuX/2; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=BifOLTg6; arc=none smtp.client-ip=103.168.172.151 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="IJjHuX/2"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="BifOLTg6" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 07D041381E13 for ; Wed, 31 Jul 2024 10:15:00 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Wed, 31 Jul 2024 10:15:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435300; x=1722521700; bh=Si9etyY1Cm I533199jI1jEP50fsbX2gipMnoIxbjPR0=; b=IJjHuX/2iUB4y4hqtgP8OuhFFl B4YXHL7QC/15eqERi1SZNGu/qfmdQsONlnsvRYVvy/57FUkQ8utlovrHTOdIbKli fc2pXPMh/7aLZoPvkURWj+EsT7u8yFgNpDrccGTePyOsA4L6SEj91L+eQiqRdZ3s hQq1L7nIq2XLPSD45ZwROWlNn+VmHb1CRaL1AwJyT66zXJpU9xwIgySEDgCrFzn6 bXRepbRa1njx1e2wSmuthMXrw0HpupSGyAe1+TzIczaDsNw6aDcghbKKxfpw3EKS fRHYQ5d9+MZDo5rfSBuDhqbWYYTZqvqj+LiQ56ZmiINax0n0fQ7Avy55f4/A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435300; x=1722521700; bh=Si9etyY1CmI533199jI1jEP50fsb X2gipMnoIxbjPR0=; b=BifOLTg6kdc1tTXPOeKImM9eWlntrUKFJmvQBF/YCqFf iEWVBYG9ZL/yZ6ROrxmUxO7HSs8XVmoJN3SjBa8BZC9bGxuGeu/4eMyT7sibc0cZ ca0zxVXz9hbbjgvIEhMBNHc+unmRSv+4LOgsRGJWugToomy4/I7f8yDfBpUuKUCC 8k1vD+jZz9oVj6MlhfB49Rh5ydb+k+BZLbdLYk9KpuHYYANQrEEu3Xlf4iNo95Qn z5fuicrwNIC2Mm65azT/QJTigvWmicqrCClULfrLe4n+4icOiM7AlrQiiszYBv5+ nUH0Cxozs5Tnexg+sqQA3ItyMIoV4DTcyKtMLzrWfA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:14:59 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 971f2713 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:29 +0000 (UTC) Date: Wed, 31 Jul 2024 16:14:57 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 2/8] reftable/stack: test compaction with already-locked tables Message-ID: <123fb9d80eecbd3690280991e0415cbb718b7202.1722435214.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 | 103 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index e3c11e6a6e..04526c6604 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -863,6 +863,58 @@ static void test_reftable_stack_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_auto_compaction_with_locked_tables(void) +{ + struct reftable_write_options opts = { + .disable_auto_compact = 1, + }; + struct reftable_stack *st = NULL; + struct strbuf buf = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + + for (size_t i = 0; i < 5; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_VAL1, + .value.val1 = { i }, + }; + + strbuf_reset(&buf); + strbuf_addf(&buf, "refs/heads/branch-%04" PRIuMAX, (uintmax_t) i); + ref.refname = buf.buf; + + err = reftable_stack_add(st, &write_test_ref, &ref); + EXPECT_ERR(err); + } + EXPECT(st->merged->stack_len == 5); + + /* + * Given that all tables we have written should be roughly the same + * size, we expect that auto-compaction will want to compact all of the + * tables. Locking any of the tables will keep it from doing so. + */ + strbuf_reset(&buf); + strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[2]->name); + write_file_buf(buf.buf, "", 0); + + /* + * Ideally, we'd handle the situation where any of the tables is locked + * gracefully. We don't (yet) do this though and thus fail. + */ + err = reftable_stack_auto_compact(st); + EXPECT(err == REFTABLE_LOCK_ERROR); + EXPECT(st->stats.failures == 1); + EXPECT(st->merged->stack_len == 5); + + reftable_stack_destroy(st); + strbuf_release(&buf); + clear_dir(dir); +} + static void test_reftable_stack_add_performs_auto_compaction(void) { struct reftable_write_options opts = { 0 }; @@ -911,6 +963,55 @@ static void test_reftable_stack_add_performs_auto_compaction(void) clear_dir(dir); } +static void test_reftable_stack_compaction_with_locked_tables(void) +{ + struct reftable_write_options opts = { + .disable_auto_compact = 1, + }; + struct reftable_stack *st = NULL; + struct strbuf buf = STRBUF_INIT; + char *dir = get_tmp_dir(__LINE__); + int err; + + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + + for (size_t i = 0; i < 3; i++) { + struct reftable_ref_record ref = { + .update_index = reftable_stack_next_update_index(st), + .value_type = REFTABLE_REF_VAL1, + .value.val1 = { i }, + }; + + strbuf_reset(&buf); + strbuf_addf(&buf, "refs/heads/branch-%04" PRIuMAX, (uintmax_t) i); + ref.refname = buf.buf; + + err = reftable_stack_add(st, &write_test_ref, &ref); + EXPECT_ERR(err); + } + EXPECT(st->merged->stack_len == 3); + + /* Lock one of the tables that we're about to compact. */ + strbuf_reset(&buf); + strbuf_addf(&buf, "%s/%s.lock", dir, st->readers[1]->name); + write_file_buf(buf.buf, "", 0); + + /* + * Compaction is expected to fail given that we were not able to + * compact all tables. + */ + err = reftable_stack_compact_all(st, NULL); + EXPECT(err == REFTABLE_LOCK_ERROR); + /* TODO: this is wrong, we should get notified about the failure. */ + EXPECT(st->stats.failures == 0); + EXPECT(st->merged->stack_len == 3); + + reftable_stack_destroy(st); + strbuf_release(&buf); + clear_dir(dir); +} + static void test_reftable_stack_compaction_concurrent(void) { struct reftable_write_options opts = { 0 }; @@ -1016,9 +1117,11 @@ int stack_test_main(int argc, const char *argv[]) RUN_TEST(test_reftable_stack_add); RUN_TEST(test_reftable_stack_add_one); RUN_TEST(test_reftable_stack_auto_compaction); + RUN_TEST(test_reftable_stack_auto_compaction_with_locked_tables); RUN_TEST(test_reftable_stack_add_performs_auto_compaction); RUN_TEST(test_reftable_stack_compaction_concurrent); RUN_TEST(test_reftable_stack_compaction_concurrent_clean); + RUN_TEST(test_reftable_stack_compaction_with_locked_tables); RUN_TEST(test_reftable_stack_hash_id); RUN_TEST(test_reftable_stack_lock_failure); RUN_TEST(test_reftable_stack_log_normalize); From patchwork Wed Jul 31 14:15:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13748834 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (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 B3A521B29AF for ; Wed, 31 Jul 2024 14:15:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435307; cv=none; b=kFHoqPsXxFCirxlY83i9S6ovT0K/k4QBwKN+eIW6GUqkp9UPBZvrHsLiMZJ3qOgM2tbIndGlEGfIKOinhVOCcaYProBRevNUqqhQDm0DJsuKTZiXF4Xu9cUZIcSXHbIm7+1244G0+Mo6WA8eX9mA3DKky4Wa+6XqzKxuh8voATA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435307; c=relaxed/simple; bh=fBRaXFS8MPvnHhcR/egteD/EyGRCgv/KauuAx2Sm8Vo=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZCpk8WUGqeQfPJc7RGX9oIKLxwrIm3WZ97GsAVUzShaNqlrCAkQK54DHReuWlVTFCfzk+Fkgz3bP9TLbvoOl55/5G2jhhN2ms9oPngJ+++gpbxQCFg3b0CzQwTqYtwm+frDqwVj6j4xYoqdhoc/PgXYjg+jRw55BY4O6OB+gdeM= 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=Ri9Zb1ej; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=nXx3C+Fu; arc=none smtp.client-ip=103.168.172.151 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="Ri9Zb1ej"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="nXx3C+Fu" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfout.nyi.internal (Postfix) with ESMTP id D30551381E13 for ; Wed, 31 Jul 2024 10:15:04 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Wed, 31 Jul 2024 10:15:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435304; x=1722521704; bh=5XM5RP7Snv lBYI0Jc1VH7d2jaLrnMpEW7ihYBko+nts=; b=Ri9Zb1ejLyb3kfOFM+KUh2R8j9 7bPawhjTdIh8VmJwtXk8Mr68yh5UHioad4kbqAQfKcbRi/xCUHQzESy8d9DpBAN3 9aWDQO9kQFmUxP2emJntU7LBZoevsZAVeVYEsiuc3MWWK0XPio1C+YHzTHFzZop6 Ikt9L5ze1PjMyhSiYS6yJrhmX1stEmQLxjRUokqoybClat8nemZup4yRed6gPQy5 bqHnq5Y6npknEXUFizqHvtOKFwCxOT8gRKV5BsxVWu4o5Q1AYY3C/UZiCVq/Z9Lw GR7mSvFtTvfsJInBqP5mVSZ06Xi3Iiw9zX/4V3QLdoaILfuJvep2HUBMVfIw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435304; x=1722521704; bh=5XM5RP7SnvlBYI0Jc1VH7d2jaLrn MpEW7ihYBko+nts=; b=nXx3C+FuaN7gAdD/ElLsJoV+433aBFrNzwojub8P7I2K J2JbC3oRZl2f73a10kYCh6LolvQJy+DVeSPqbaEcZ4NTQPHSJmMQaVwrvmysOHod mzGtFIn1a42q4Gajr9n5SNW6oe9KA/uwRX2sLuegHINyP6mczRF8GrJLo15GYaJf TowdN5kMgVGuT+OOvh86wNfL8nzI9oMa/5S4Vm5/zNLxfP0N3r3t2MKJJPTGJK0k 0wSDwlwP7vqJqXf3Ybj9SClIdf1rcx+253J1KmgYpf6XlYFJTSPbUPOoT7YBsew5 Sk+RCspKGR4GcVrI77YCL/k61mgXT91OM4+gzrOfyg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:04 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 495e22cf (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:34 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:02 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 3/8] reftable/stack: update stats on failed full compaction Message-ID: <1fa7acbddfbf5f195047bb99ca7312f74a897223.1722435214.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 04526c6604..8739ff3d63 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1003,8 +1003,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 Wed Jul 31 14:15: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: 13748835 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (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 5DACB1B29AF for ; Wed, 31 Jul 2024 14:15:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435312; cv=none; b=oim4YiQXAjiCKXhlxrRvnHX99qYYZjPV73PKFE94ODBXr5C4mCbz7d1+BGoaEI/4x4zCEUXzs/6snGFwonaglS4mfdfIx6p4qDqDTP1iw92ZahecRj0qj64ZtKEfGsdgRcdh1yr5grSVxpMjYmPpNtyEHLg9OvlbnBmsU6BT/8k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435312; c=relaxed/simple; bh=ndNjw8mUgbZEU4uqRMxX7J14L2Nd+9SZJrzEVSw5FQQ=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LvLtFNAyuX+NGYs66iUtGj79QoEHwtYLZ7rNPxbdFwylLvUUh1Dj8x4/N5eOPeRQVhPGvW+K7SWrxgqpSsOPMu8HkSKgJijFR4tuGE53KFHXFM5l8Ccf2PnnO3hJ/BJImbFm6XDMYcH1NHIVZav5ZDoCQnaDHy+iCuunOzGvTJY= 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=qcWUnh3C; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=fRF47Cnl; arc=none smtp.client-ip=103.168.172.151 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="qcWUnh3C"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="fRF47Cnl" Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfout.nyi.internal (Postfix) with ESMTP id B2237138210F for ; Wed, 31 Jul 2024 10:15:09 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Wed, 31 Jul 2024 10:15:09 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435309; x=1722521709; bh=rscWh81Ay/ VFR3wNsJ1glfvD7RiTQZlUK/hNkJQFgZ8=; b=qcWUnh3CpHdJ1gLtOvWZsltL7A Eis61x0ospCTeWuU+y2gpLm3bgY+DOAZOdUKJGuBekWeGi5cUVW0TOEsXZ9nZlvJ hUqZocG+pYRGAv/HEhz1+YOqRcdOfQRLItbJvfvcirkaERB4X03KTRQ9oaEEnFLa vKjC2Qrk/lsmVF9yYPfpmxkdhVkbsOTsNebQcOT/GSviWlvWL8f9OqlZIVyf08Bz k0IFz0i5sb5QTmggDZFoyaIZoa8HEUIf069L0WozUxWQk43kbEcRfGr/PdAM2oJ3 n0ndIWCps6SIsXeDnRVAQqb9bjG4O/mTQ7kUA0pOC5hfFCNLV/iXmxBWl4bQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435309; x=1722521709; bh=rscWh81Ay/VFR3wNsJ1glfvD7RiT QZlUK/hNkJQFgZ8=; b=fRF47Cnl/PProRIvxdDprZKP/iHCVQXpMUD/6vz4o6QW EH5VeZ0WSductxwOGxkXgMB7BuiVFibqyXVKIuw2x/2Rht36w9Dag3Fo6tVCdWxl UXiHzE3SJoirSu0ff9LLew7iFNNfSDL+//FMHemvjYuOBl49t665CXgw/pKkD3Yc 245/TUc3/0GIwF0IKw421BdhXZ4tUeTngSUdoEWneo40tSXCv4VCe1O69MgvplqR UJZgZpQ3cfHmOiGOwnxSgdoXWR8/wfvZ4WYEZONGyNMsb9ChSD/ykMubIF9DkZn0 gXrHPqNGwK+NHt8XuUC6LjFefd+MWKelNA4XZG9BSw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:08 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 8c9d40ab (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:39 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:06 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 4/8] reftable/stack: simplify tracking of table locks Message-ID: <40d9f75cf20d4b76adb1683709e054e264d4e06f.1722435214.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 Wed Jul 31 14:15:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13748836 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 D9E6B1B3F18 for ; Wed, 31 Jul 2024 14:15:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435317; cv=none; b=tXgVwqCBZkCV0iM1qGJKp1gA07lpGm603YhRhk7nybqCmqdoIZYx7b4LzEVjF8X7+G/31OmalXoYHX/bJwlDQkagdNVkTHCLWd9P0QWPwMpM2Aab6aK/qg63OqGl5c7Pi/KqRPH1LaWFr4krpgWvdYnEekdnpZt1X5XoeEj4+BQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435317; c=relaxed/simple; bh=cgfNmv0J4ZDCut5gq9EFCnl70+1Yx6fuyWl3ByIKr/w=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ufnYSaW2BDjTwNC0DwMMttBnmFsWpVbxqYLI2RELMR8rvSR1/Z4EWelM1CFo8U9Y6qzDnYFzLvieA6xKxF+NBro8JX4dP6RsNbWZkFFmKw5mQR4Ddr9gS/yajPS1fpvZqd7mLdtjIO8djGbgNAd/nr88d8uDRLoU2CMkAPTuGVs= 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=Kr4NaAsX; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=hSTJfylo; arc=none smtp.client-ip=103.168.172.156 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="Kr4NaAsX"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hSTJfylo" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 90209114061B for ; Wed, 31 Jul 2024 10:15:14 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Wed, 31 Jul 2024 10:15:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435314; x=1722521714; bh=nNKBzdbZsX DHAHbYWCRNWwfre2rwOy3tplxWmhXiMsA=; b=Kr4NaAsX11qsMbxx9EJjKc0oL5 kDsy7URpruck8TU1t82aoKmoOQDkIocDfjnTp2yP8wZthdpx199+/98SesQNpuHt gsEZE9LHm1fKPuN5L3g7XJEM5sVvB/XzzN6cmmRXPxG48BcvleoWjb3H246pAK3d xnx095BFDVY1qK6gHrocdHCVfE+nODW9tFZ8q5KfNvqGsPzLQeZRp2cULo95yUt6 LnlcbbpS3JN8hynts4PHYQfnv66I7oJ6H9Kj7FV+E+sTR20SBs48pGDbZmxMm+6D JcOBJBYCb19G1b99Ui+0I4fGLOw81SFu3/kBdVBUL0JEv9XdCiXjfGrdx94A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435314; x=1722521714; bh=nNKBzdbZsXDHAHbYWCRNWwfre2rw Oy3tplxWmhXiMsA=; b=hSTJfylo4IaR/graPCpg5pd4tdV4DHkb5kKhQgtQGXll kAzGdOq9aPmM8sT/OgNZfpx0SSbwyx3owBIlsp9Jym5/jg+ICpCPQ7sGldpNc5mA fbBZxgmVEJ/Fv6DlUI4lwI+bRpiEaiMrZadNdyA8Qt1JPxqDB39dR8K7WbHOunAA Rg8Ul6BVdyRMqLEeMmHdO8fOI6VAYo4I1sTlFk2EiyCRWFy86ZCX+3noYuPJ96eT awa4NdKRGH+aMltcRc3AaljXXE8GaX/8tnYQTy871XHOa9+b8llRpUAASEA4gYft dqrSuT8eASj3shzQAhQqkW9q71OIIrbGiqiQ1AhLTg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedunecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:13 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 714f9d61 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:43 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:11 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 5/8] reftable/stack: do not die when fsyncing lock file files Message-ID: <9233c368941ebaf52f0aa9cd2d16c4334e8eefda.1722435214.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 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 Wed Jul 31 14:15: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: 13748837 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (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 EC9591B1512 for ; Wed, 31 Jul 2024 14:15:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435322; cv=none; b=V0dbu6Y4oQ1wNroOL0fTCuCgpboc7q7vptZnDhtkbIP9mtaOUy4rnJ1K88xuwhPd/GO0WB3oU9EnGWlDJQjgocLngy41v/U8WvRzF94rPSjVtHKCmdkIitu1WB+ENrv0RLX0ENgOWpT7fW6+Hnhm8mEaE3oaPPvyKl0QVaHhzzc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435322; c=relaxed/simple; bh=+x4xHX5edfHpQX+vj2n6XFJJRzH9zhxGX4ho8aKMwEA=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qH7v30AtoK0EkwX47swWu8Es07rl3wFtOXigzYXjz3V/CnzKGjMix4qwAxE30zjLjttxODtr2iWDLcz6Ist7QNaCEyk39hQbmkBDrqSEfU2KUotQIEQ+lttFnYeDGuYi1dF5ZaKsE0MFQWBzoISq1IGB1cb6pp1iqQZWTnYAhYM= 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=UKhllHAv; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=t/lbT6mg; arc=none smtp.client-ip=103.168.172.151 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="UKhllHAv"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="t/lbT6mg" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfout.nyi.internal (Postfix) with ESMTP id 5E714138215B for ; Wed, 31 Jul 2024 10:15:19 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Wed, 31 Jul 2024 10:15:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435319; x=1722521719; bh=tRTLmq+AjB oBCztpJO2rvaf7ZsfDOIGoj0+C0AXkxII=; b=UKhllHAvkbGhXGxpstTOCY7Mfo OfbP1c6JFxViTwNxlnimQIApTO1+SKVpMoGgybzlAPIHm8Tw93JWN/Mu7phtz8Pk o/Mg5NEDd9bi0y7LTzzyvzR3uVY+vqitzkb0eXx+qoCTM5b+Wygy76Nz8JMLlmcY 0HNa95x5veB8uM5ISqeLR1JyUDqHzSQ6ti1eik2uYauX/Te11LPy3IyJkr3+7hSs rdwcnscuMuGwRLOb6WbyAGVrxJeppHXPdTqKc9FEUASGcKChaNh/Gv9qbZa/TBoc mCHC78NUOytlwXMpJWJGnXGZN4M6b5WhSnhOPNfQaO5DWNrvl39goyDGnpog== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435319; x=1722521719; bh=tRTLmq+AjBoBCztpJO2rvaf7ZsfD OIGoj0+C0AXkxII=; b=t/lbT6mgIh8YtrU/JAilc62d/W2YtoN+lJUAYC/WIoyw h4IBQ3uvK5nIOLeXwl1OSxOXZyEh/CddK0pIjf2GGYoBET+TRy/4V9KSDejWUhdz P5TTudP9AqoT6cty56Mn6CKNcKKdU4F3IsZCy1ecag9ET6PjHTrgMI8aCHsfAImh TaFUzkFi3fnsLAUBKoBVqFHdVnoR8VID+eKzJq1Ht4mFZVUTuVAfEzlMfqz5PPJv U2YbRIKTdURDaXsLhXMIdqpCFAjgp1pGz5M1ht20+1XZRR6p+Cq3oguidabj9k82 T1nCpnuWz2RVQSQ1Mf1CciqItVnJJfmw+7IoNaKVeg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedunecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:18 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 70f74262 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:48 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:15 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 6/8] reftable/stack: use lock_file when adding table to "tables.list" Message-ID: <9703246156152e1c8c92b4237c8dc3a9ef59901a.1722435214.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 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 consistenly lock "tables.list" via the `lock_file` subsytem. Signed-off-by: Patrick Steinhardt --- reftable/stack.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 9ca549294f..9cc91a262c 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; @@ -674,13 +675,14 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd); + err = fsync_component(FSYNC_COMPONENT_REFERENCE, + get_lock_file_fd(&add->tables_list_lock)); if (err < 0) { err = REFTABLE_IO_ERROR; 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 Wed Jul 31 14:15: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: 13748838 Received: from fout8-smtp.messagingengine.com (fout8-smtp.messagingengine.com [103.168.172.151]) (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 D8B241B4C29 for ; Wed, 31 Jul 2024 14:15:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.151 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435325; cv=none; b=l56TQyqoYicM9nuCca0y2WUIcbRjwdRxcNSLoQnCZD+J/BPp7jmWWgHu6alnmTsXeMp/yyN/vjNSjSiOQWEls362INIvI6jRPwxKNsS62KTZlMJkDWuitaaw4vuh0DfmbJaOIfRoO+W9oCtto/BnpbRy/y3vVX7UDwynIbEUW6M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435325; c=relaxed/simple; bh=O34PZl8uT/u6hMqOdbe/xJNk5sgOyMqz+2BpMxGI3iQ=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ptYIUOodQC7q06Lantdhxho/vPk35Xa0HW3x4UN4EIHSYLl4vogDIJCqhblUaUgKuMI7llhEHsKY/3foLAarV74HfdWkECJTfqd8RGzQaIhBvKOtTsd8EEN4UHt70ORMS4LxJJCVUcEY87hMaJdsalO9HFxS59GDEpRQHVSTvQQ= 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=gxzQUyR7; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Kcqd19QW; arc=none smtp.client-ip=103.168.172.151 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="gxzQUyR7"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Kcqd19QW" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfout.nyi.internal (Postfix) with ESMTP id 37D471382135 for ; Wed, 31 Jul 2024 10:15:23 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Wed, 31 Jul 2024 10:15:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435323; x=1722521723; bh=Ffn48IIzr9 k/H28o6JaqtHKAHOFvpbIFWked0Dplr+Y=; b=gxzQUyR7ObNhR+HN5vV6eH27H4 VC9uBc7qybvuiptCvO49Up4apT20UyKbJ3wcsjUHb18c3mP1vWxGIXpr5rXOSU6a PAiwLmJzDfPbzcnUP1CUs9r9UCcDR6ERIFxJJ4gjpz/Hy+ljMqyGyrH737C8gQmU 82eywwIPIzn16KMxAVmwf2I7zYxUoFGooaUEbTMeF3kVmX44CUmWKc8iJ/RGu0W2 KGRyPaF9c255nJEKt6x8OBo46G/R1QeeDqvJNr1iQ0XaNVCRVFkG42HNv7zi1pGa mRGNEwvhRiGmpttA++5FSAsdclKl5JGlh0okZYAm0l/JqLOxoaBv4r8XfEkQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435323; x=1722521723; bh=Ffn48IIzr9k/H28o6JaqtHKAHOFv pbIFWked0Dplr+Y=; b=Kcqd19QWjZs7UUEFsDZ+Nrc5nS3nkpF3cWpRmySgXV+i Haob1NYBdOPCVcZmF0RUxkujMj4BU+TMW+0xPOjM1WLVPp1wn3bqBRjXN91ebBgj wpnyCfQiKxkBsIP8rPf8Jb1QVw5tJHZSitqe9R+rdSxS0NaONTBg0TkqauiSaxl5 vg+0boJZR9EbUMfKbjUm6S3ZGasCvD0wJHXYGugADQpED0U3T4Z8sZJJcUVPN+37 1k4eBsy41wzj3lyuXhDMSRZlfsHiwjLT6L6vYuc1ZLVy3RjedLR7/GSVbl46rdaC J73F/s3/t0K+d541NKKNiYDSdWLRwd9vkXGSnP9aCg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:22 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 507202cb (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:52 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:20 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 7/8] 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 or 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 | 101 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 5 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 9cc91a262c..2b1ac58120 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1021,7 +1021,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; @@ -1124,6 +1126,94 @@ 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 { + 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. @@ -1146,12 +1236,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); @@ -1204,9 +1294,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 Wed Jul 31 14:15: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: 13748839 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (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 D61531B29BB for ; Wed, 31 Jul 2024 14:15:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435330; cv=none; b=t4awThV+sl+L36sl+6tbUY687Q50oDvZvWpRbHYNVU1Fc4PiLsC1ADu2W4UStTD49AcPRJ4QfF0dGSI+WYf+k/IXnQm1NLP8wGwEdgUiEB3+x52UPT+Px4OvDc5Kh+1nko2NEOT9ltP2g7FxeMKqBEdBf0o/fUY+MDwPwZLovGQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722435330; c=relaxed/simple; bh=lZRSt/dG70/PRrEGpuMnUNfa70upEdtfoIunZu027hc=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=U9gOiMWfP+5QQq3GdGZy2xwjscBXnP+JIV2P/2jtJs9G0bpaA/NEVgyjAy6FkRPQhG9tI5LywNEjg9Jwbn3oa/NjUjkmqMaDGCxadBXn8MRba8wR41UqBPaPdve2ZEKcoW1guWkWSHcX9u8Mq5GMXUwKPCMMbAAywCNW2yIenuE= 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=cecjgH99; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=i9AkgJv0; arc=none smtp.client-ip=103.168.172.156 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="cecjgH99"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="i9AkgJv0" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 09AC21146C8C for ; Wed, 31 Jul 2024 10:15:28 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 31 Jul 2024 10:15:28 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=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=1722435328; x=1722521728; bh=vyX6543lL5 +yNmtZ8+OZPfcXOj4USEf1teePG8hvMdg=; b=cecjgH99GcmOz7H9hn2QJxTdFZ QL+QhFS358d08j59X72q9yTNHS3MHROKGINsvkcJe+bTF2n5LgghrshNLb+OvPwa cKkgmk6rxTNNHnfOpT4PTuM5CWCNkTHK3mVcjOQmcESBgW0Hbs4ScQzD/M7sGMJc 2MlKwwhOEnCXLVBYTNIrtBsxG0gvpKlRzVSOHTdbz2+Z0BdiR8yvJYl8Mdqe/zbJ 3M9f/YXl3BeY8q1YUszoN5mn60yoQaIAqv7ZT47y3sRbLt/AzFGvII5ycqMAmN3A Elqi+F/IYL0AZud5ggo3ZTU/U1N4L2bI9H97SLNlMEYRTwX3bYSqec0RQnow== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=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=1722435328; x=1722521728; bh=vyX6543lL5+yNmtZ8+OZPfcXOj4U SEf1teePG8hvMdg=; b=i9AkgJv0hcm8AL4G8SxnPyFCWLbnhmsqAGRe2cilC4kv gpb+CG4//0rl91xS621tUnLWsVKWQPjV03PbRee8BCmlnKS3RPy0QmlUdm+kNd7b 4PJBgwCjLinWn26bU9iMgx8/+8HW66eHHKrw0/Lf7hnToAk4nMl3yR8j5zohPouO UKI3xePXU2MO4Jip5/e/HQ2jFA1KPyMgdN8rZMUAg6LXj1305mA5NC7ClGuMcDt0 3EYm1z4jD/5TcrVA5WQb6SeVpb0bb8txRJX+AkuCTEs0RrVMaSKHDG4bT0RkwHcb CEEblR+32Ks1XKnJLVwor9ay/nCdrF+Jdd+mQAoKFA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeeigdejhecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtderre dttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhk shdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtdfhvd ejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgr mhepmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Wed, 31 Jul 2024 10:15:27 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 1553a4d8 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Wed, 31 Jul 2024 14:13:57 +0000 (UTC) Date: Wed, 31 Jul 2024 16:15:25 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 8/8] reftable/stack: handle locked tables during auto-compaction Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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 | 61 +++++++++++++++++++++++++++++++------- reftable/stack_test.c | 12 ++++---- t/t0610-reftable-basics.sh | 21 ++++++++----- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 2b1ac58120..9657ca4418 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1000,6 +1000,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. * @@ -1011,7 +1020,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; @@ -1053,19 +1063,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; + } } /* @@ -1270,7 +1308,7 @@ static int stack_compact_range(struct reftable_stack *st, * delete the files after we closed them on Windows, so this needs to * happen first. */ - err = reftable_stack_reload_maybe_reuse(st, first < last); + err = reftable_stack_reload_maybe_reuse(st, first_to_replace < last_to_replace); if (err < 0) goto done; @@ -1303,9 +1341,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; @@ -1315,7 +1354,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) @@ -1422,7 +1461,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 8739ff3d63..af5757c0f6 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -902,13 +902,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 )