From patchwork Thu Aug 8 14:06:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757509 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 CC40818E749 for ; Thu, 8 Aug 2024 14:06:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125985; cv=none; b=kp67V1QqXIboTlnmU3SaOVhnL9+0vSm41M8EIJgx/VkgRqNYsLSUDpVg9rvZtcpYLiyy8PB84218KmhIUh+UokEROhnDjzI31oiNdOR1NHteF5QjS+7MJMszKINSkxkvNrr17gAYEUXs6IAPrRj087WFtSqaa0mgs+ty5IgEw2w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125985; c=relaxed/simple; bh=llTkx/B4OIpnbso2sWqEJgYV9Su/yABh98HDbrbSRz8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uVpPcPBKh/IQUCJleKQqad18larWfPl+CZfFr9hdaFGjQ4NShI2jj922uyebFNfnyM+j+6v0aS4z0971K+Xzy/JsS00FlhKlP1Q66Pr6Syf7bHni/yUBncn8QFNm7B/thI6y2ionpVRhHCs0n92+lHVAE0NnS8MBc4ESqRyVeNI= 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=HDvV7Vp4; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=P8vI5xjB; arc=none smtp.client-ip=103.168.172.155 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="HDvV7Vp4"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="P8vI5xjB" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfhigh.nyi.internal (Postfix) with ESMTP id C109511509C2; Thu, 8 Aug 2024 10:06:22 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 08 Aug 2024 10:06:22 -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=1723125982; x=1723212382; bh=4NjMY1h/RA ppN0/7zb27xhG049ctrstRQasmLNjxHpc=; b=HDvV7Vp4ZndKFU1i+Z/5UIDZ/m MQEhcu6+7M2Ytdkjoq14OgMpAx/tpIfv0Ht9LHXAxvNzSSJGEu0AACpqu3Owx8gR Bzt+DyNxCMGS1BGwQdz9AGFLTDFkzig9wRA5EbVPbxHuqUXQYH9nBFRg0qydQdDU cuS86vdo827W2puARYlshLXaG6B5CGTujU8bZWMatVYlF8hzOi1pboJyhnqBzKJU X9E9XXMuhSMYOYFEvNJbRHDnbMg+DO0XOr9fc79cxB9By6YQiBY4iMJ6THCWcI2S CwZxrUTynjYpmttjKA9swklt8wG9UeBDAJnrhvLyNmljqjc/mAH/0DvUNXcg== 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=1723125982; x=1723212382; bh=4NjMY1h/RAppN0/7zb27xhG049ct rstRQasmLNjxHpc=; b=P8vI5xjBTk4xJaXvztfST9nlLElPfN5KsrO7qQhEi1pj Ir+UwOtUxsfhRB+0VEUfet5aTptysmncVkZ2C7tIWFw68qXIRRYXIPo8YUojYJWz AieDUYTJpUgWI9Gz3AtSxkaSvSTKqytvK/jy6idVfn7BdnkEojEtgRy6fhRBJxEx mc7LT3UA6aoEZXI37qFHlzRMSajLSDzNX+JTnGLEmYSh0V6PRaDCLB3xcJKFsb04 4gO34aGzDcL7uQJtmcz57biC74iUIUaZx+Z2udroM4q9FYxgk8KJoGdbxH2F5Sy7 cr/DYieaAMby2NND5Y81i3zLS97NmXSjz4xoHBRq4g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpd hrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihht sehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepjhhlthhosghlvghrsehgmh grihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:21 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e974f05c (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:14 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:19 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 1/9] reftable/stack: refactor function to gather table sizes Message-ID: <6d2b54ba8e448eaa2584976e1fe73c798842833f.1723123606.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 Thu Aug 8 14:06:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757510 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 0C03118C33B for ; Thu, 8 Aug 2024 14:06:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125990; cv=none; b=VtzpKqnohJXPBo7V3lEEZYT9Ye7mkpH9eEFzedzlr41BY+dmTLAA572QGom4GBofWHhyLUdQkyEcr4cBbxR6D4hYSEZbOColFeQNIW1o6/Oef7hdDdb2jf/cnmE6BoBGCBJz/mPhGLn03VNaUwyViB10G6ViJeHg7IgZM69dX/M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125990; c=relaxed/simple; bh=Vd7rfmXPLBH0DsiVDfHkK1o8aPw4euFlYZ7rIrKnSiY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Bpq0kopelgxKl/SETQnE5J/WlXu05rMdph9ogdfBpRO/a8xbiQGJEMQmnMVXICyZWTfyC8u+usrVgpy/kGHP3jjSI9tOrvKr4o3nPcVOmi1cCSA3+SdMWv7bTkWwDEeUOE+zEjhFULAfOiN+4SSgJMXUZ5vxtODv/DV/kR4ZuqU= 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=FvrXRG0M; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=iBP21AAL; arc=none smtp.client-ip=103.168.172.155 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="FvrXRG0M"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="iBP21AAL" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 0600F11517A4; Thu, 8 Aug 2024 10:06:28 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 08 Aug 2024 10:06: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=1723125988; x=1723212388; bh=mYMhjsUsxE 3oB6YPAnVKvVYgNUtOqGe4p2GGjMebQ5w=; b=FvrXRG0MHGXUreCKLUM9whcazk kiiGjbg/yyFMK3+pPgwWfbyDLzTol90NTi/KQMt9O/62kUgNoEh5aESgdaSnJFGq cE81WYyBp6CZFCO/ER8CmMegDG0N02qmbvR+V4A5rbZx1glNge/jeDx8ifNhywne jY/X+JhNxxUGeRLNHdvuqxHvF8lQG5Oad+fnuO/7FOsWJWZSjHAvB7/Cdzg15Hdl jshHXjE96HW+XMXpk5ITzf++z6WA6ucl68qcOl5l3k7BoyvfrFuP1LgIW8wVnKfO nWVvLaqn/wqgQLGlgs1ADxV2OVTm6QcOsWctnt1/Jr4YtGmeVITpAvFur+Fg== 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=1723125988; x=1723212388; bh=mYMhjsUsxE3oB6YPAnVKvVYgNUtO qGe4p2GGjMebQ5w=; b=iBP21AALFgKkWI5/9i9DyhR8rYbHpra0uKNmUNEyFG55 nT95Rt50NMNCUYehfM6bje2cy1mRUnha3hzUgk+5+v1dOYrDYbHu5L0JcH/yzcls i+mD1I7dMP7d7RKRLyoabzdWWicsm0Qx/HGEXSuf386QX1mRG9ma6dUS3+CKO0uZ EutEs+lEgSo8x6NBbP14Cz+ctcfW/oMqDsgbnknk+iYAqB66ROQhNwyLtW8OBObO 2cN1iRLuF7/d86NnKMv8MgoPI5lnbqkH7dRs19rkgCLlmYL4A6V6sO9ebK/qc6Dh r7L6SyX0hDjN1j6ZlqNnJ5jRXWnKkRFap7TvfDcOzg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpth htohepjhhlthhosghlvghrsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithesvhhg vghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmh grihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:26 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id f713fedf (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:19 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:24 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 2/9] reftable/stack: extract function to setup stack with N tables Message-ID: <798a661824cfaa93160636e91b745b57e2d52776.1723123606.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 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. Furthermore, with this change we now start to disable auto compaction when writing the tables, as otherwise we might not end up with the expected amount of new tables added. This also slightly changes the behaviour of these tests, but the properties we care for remain intact. Signed-off-by: Patrick Steinhardt --- reftable/stack_test.c | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/reftable/stack_test.c b/reftable/stack_test.c index e3c11e6a6e..0b110f6f02 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -109,6 +109,34 @@ 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, + size_t n) +{ + struct strbuf buf = STRBUF_INIT; + int disable_auto_compact; + int err; + + disable_auto_compact = st->opts.disable_auto_compact; + st->opts.disable_auto_compact = 1; + + 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); + } + + st->opts.disable_auto_compact = disable_auto_compact; + strbuf_release(&buf); +} + struct write_log_arg { struct reftable_log_record *log; uint64_t update_index; @@ -916,25 +944,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, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); @@ -965,25 +979,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, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); From patchwork Thu Aug 8 14:06:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757511 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 0F5DB18C33B for ; Thu, 8 Aug 2024 14:06:32 +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=1723125994; cv=none; b=q+6LzpLAdmSXbbHRcBs2Qog6uJBUh9Y6vdxuT0mnXaS9qMG2CTobPhPoACeoUbJem6NDs/3jwHAC6BJ5cqjudM77/mCXyEU9BdDgqc0nZ0oxitff8gOgShi58XQNk6E7Kawvw2ea96C2MBOzqqPDsKk6XWPEE5JPWzkhVDbKQbc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125994; c=relaxed/simple; bh=TOFwokuvQxqczzilN1271PZRXKPZ7Ct9VhhWnkF6oYI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LWUIFy7kuTs8Jt3eIgAPo+sQSElqNrsVCWO77CTwPCwYcjO9wAVROjDjEGtSf+kOKTTgxwJ1g8Lrm+yReCyBu9nVAzZb7A2MC8mZEuQmvHm5LDyeaZLzHEJcRK0gxj+n7FcIKzLzAg74p6zEv6b/r3oIbYQ406wDlvBjxWiM2YA= 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=guWtGSki; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=XvjK9BeL; 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="guWtGSki"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="XvjK9BeL" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfout.nyi.internal (Postfix) with ESMTP id 36568138FC67; Thu, 8 Aug 2024 10:06:32 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 08 Aug 2024 10:06:32 -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=1723125992; x=1723212392; bh=GTZ6SofbRg MJZBU/6GhcZ2k1/PLoKrTdGw2+WGtCsfE=; b=guWtGSki5Z8TRxkjm7hrdS2vrt C+a8h5xkNTuRLrfgUliTv744KP/LEF7Vcvg7ixagVp1xB+/Xg0fGCW1fTjxzBUAL biqkp9DoA9b4F+CJIxfEAcJ6YZR29lvcC9c4OonjQVCY8LcksH+7MqSmwO5aPtdX b+Wkl+z7pg4qJ8WVMKv3DLRzFu8xZT4x62UGNkJAm4JnuK08FVMTnFMO20FgLxMv agbIm8j3Jr+rrLNKJZP793YXz9W467i2COFI6c35Ay65jVYev3Bz7jMs4JTtp+kF VZzJfDorBOdBuFYGSaVuVgj2fC7YOHWHfTczIoizozw7AlPWZ7Guh+hDlY4A== 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=1723125992; x=1723212392; bh=GTZ6SofbRgMJZBU/6GhcZ2k1/PLo KrTdGw2+WGtCsfE=; b=XvjK9BeLYhnGU/Ggma0KBKeOCRz/BFa0SqXTxJny+K3s MxVOJ8K/6ze2ltMMEO4O8YETmFdRKtJW8CY11EiUB4UvSdPzCEBvpc2BLa8MSZF0 ezUCp1s5dAaVEqdybrC+gxfPpuR1+kixwiKyevMOo4Y6ZdrfsDa9Q+2RsA8LL9lA 9Diwa71+auk8CsN3hC/2gPEKusJU2um2uk0PcT6XsjNRtdY9RQAOfktBguYlUXp1 Gax1V3Xud7k262tZpEAL/7lcPnuQIG8JyOTtC7vmQJ26L/tbTkZCeOPM2qOH3cJJ qLTHv3/jbujm9HWDTJ5503S0AdgqEYphqJaPt//FkA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpth htohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomhdprhgtphhtthhopehjlhht ohgslhgvrhesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrh hnvghlrdhorhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:31 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 9061d20b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:24 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:29 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 3/9] reftable/stack: test compaction with already-locked tables Message-ID: <949f748823d8d79cd2ecede7e3b4772e3c121098.1723123606.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 0b110f6f02..1d109933d3 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -891,6 +891,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, 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 }; @@ -939,6 +978,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, 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 }; @@ -1016,9 +1091,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 Thu Aug 8 14:06:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757512 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 6654418C33B for ; Thu, 8 Aug 2024 14:06:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125999; cv=none; b=WLsjgYusTmmC06Uzh2x6fxKkt8V0BTlv+9+D8lP6t7nyLJh/S15JsaqNGD2lfDw7xMGIo+GVgRBq4d2z8Kf7+SsHNRpWeHjTCBFjK0KXv01oZSIUkBE0E8lFrtfBKquO627I0iDnlMMKbS9GTns22L8RXEfZVDGJNs8SA4C7EJ8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125999; c=relaxed/simple; bh=SDcgO4YxoT/fbB6DbErrnOncHc+XYtLo1I5TgWJB2tI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uZfk+jIwzVorSDY2JU1X1lrn+aFYYvBhmCFFx8KfcD4tV47de/lpJtO7EHOBn4OGyC3EFdo+XH1p+BV0hbxnIxRgWnPX+DK+KsCq1q4r5EPGwkVSfBt3IXIreTeJMhwfzMHzgkhPh1fvMf70YPanVFGijXRsetm5OdEYlYUk7Ho= 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=Tj3yncui; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=c2ZoVNlF; arc=none smtp.client-ip=103.168.172.155 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="Tj3yncui"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="c2ZoVNlF" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 6E14311516A0; Thu, 8 Aug 2024 10:06:37 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 08 Aug 2024 10:06:37 -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=1723125997; x=1723212397; bh=vkKNjdC1Io GuO9vsNfJPj7MuzErxDhk3l0lBBECs+fY=; b=Tj3yncuiEbWcOu/KAb6WC/CCKy GU/rQJfyhr5kLrczfR7M9viT6R2bQyI4Q3et+iXgdtQbrCipobO5o789lkf9RTX4 2sa1mpoP/ghTR11eO6mQ20mc18UX794cb249nlo2z18VAoqfCu0Hdmd8AfgKADgQ PhW/73EFPb4WK81JvVENFSCBhCHX2otzldJC5JwQ+tu6501QI+ooQE0vopVRpfuY 0MxOy6dLlONrJ31O4cRy8Prvj6HGwjyd/MSH+7glF0xWwTiPDTEOfidVTlQZT+EW duTdhc6EmPWZUNgYhGcUldDDjmth3GzyY0ToZqYD0k6Pij1D+MDHIxppsH6g== 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=1723125997; x=1723212397; bh=vkKNjdC1IoGuO9vsNfJPj7MuzErx Dhk3l0lBBECs+fY=; b=c2ZoVNlFRcKUfVNLDdVgJLHj9W0Ch+D2iapZkDVRSl40 Vv8r1GjiQ70l5iBWAZiaDIK39X3AL/aYTdPD5lR35ZTlJ5lr0OVI+yN4c5AlKAZW SF46b/Z6B9gl6avOZJmRBvK29xZzzf5faYDJz3Ph+M5D4R5gHX4f8Ts0SKdIS/bn 2LLAoRZyl2YHwAhUuYlIDr29z1fnLgcU1NpQhqZ1pbFN/Ndz/jMejQgUokX83WIu WpI18dUm0JNPhjXC20bbMnwFAQuyzHSTzTz1CDuNi236rpA3xIA0UG1GeaYnDhff 0+VGF0U2p6Cv4jGcYp2/BpjwHf0wcaANXacDMIV6Jw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehjlhhtohgslhgvrhesghhmrghilhdrtghomhdprhgtph htthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhi thesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehgihhtshhtvghrsehpoh gsohigrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:36 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id e5597cd0 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:29 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:34 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 4/9] reftable/stack: update stats on failed full compaction Message-ID: <478f945a45dc1f5e613d5e1385f3bda5655fa325.1723123606.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 1d109933d3..3ed8e44924 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -1005,8 +1005,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 Thu Aug 8 14:06:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757513 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 8D22218C33B for ; Thu, 8 Aug 2024 14:06:43 +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=1723126005; cv=none; b=G7SguJwAgNvD0LKjxh0rtZ0JSwcdZ+CwVf90Hib1tJ0EP6HlJCMf5hCXIu0AHdHadUPxfJEQAuSBjaPOrw1eUCblZ3fO42W5jn/ECWOYpWMDKvssPJGDR+12J0I6XBdCSklA55nR8fM58uhuWMAkiTYXoZ1yrVl9Pyt4LrybJhA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126005; c=relaxed/simple; bh=jMZ3sgBxIQdQkSerdJiZl++k/hrD5XHjmuE5SFJjkiE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=W4TSkLPq0IKBUXz12vc3v0bD3MOcfGyO/fU6hE8SgoYRIl8id4PXIC5+1vv30dYdg1iSFg0NvQwXJHNdfzMWoyNaX3/HS/Ll/7LMaHHd/xt4ZczFEWo4vUr2tpUrVDxIsQU7D7Nl1ui3TW72zGDQppoRJ7a5YDtWmlkk/s0rI/w= 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=l+vTGW3/; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=jzpASW+6; 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="l+vTGW3/"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="jzpASW+6" Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfout.nyi.internal (Postfix) with ESMTP id A341E1382465; Thu, 8 Aug 2024 10:06:42 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 08 Aug 2024 10:06:42 -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=1723126002; x=1723212402; bh=3Y2Evq854Q qvq/I+HlxmYF20Q9Rw3K0VlF9giBOEUFk=; b=l+vTGW3/uh1djQh6ZzpcPqsDio KYT5GWJCwF+VBV7Z/L8YAisN2Ok4uWaNX/k/Ayt+qxwRFIz96LEJajoiEv3ZkZzJ KgnLAuV12Uw94THyGv4oawu/3Ua4auYnPOkwtuqA1k/4HoLgpPPLvvo1JLMvu5FJ 0Y/S6ONiS74JN5EtdrwJHmqSp3179tw2fUPRMuo2367u0od7IBhx9nC8jX2ID8Og 55FVwbpbbNXibdarfHZ4cAzP0jfVDw8w/SxEaq+nSIRLCYonfuWT4Lck818r+2N6 EnpGX89ftqgGzWaMOGbHXfY2tnnK0JumR4yuS1St+YG2Vu7sIK2U7YrEga5A== 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=1723126002; x=1723212402; bh=3Y2Evq854Qqvq/I+HlxmYF20Q9Rw 3K0VlF9giBOEUFk=; b=jzpASW+61UlWClbWn/hmBhSG1Bbmey32kRoq4wSrQerh 9g0vw8c9liyZZXIlJmhxh+y1WX7K3Pez6YAEh0gV3IzaYkUOcRAshVtVkz+kMNIo rlELa1bzuoDAa+C/tlCqktEnVcyjeRY9uy5abvphjw6r3fpQ+2iOgTzUbJzv7vIs TcuJuAIgJx+jiDPta0KEL3rKsmPqJ8RWc1wDrHi/ax7NNehzaXzK5jWdCRE5sjh2 yHQoaVbM7OS5a2AnFFIWcnkdJvC2cHUttSQ1W+8658wSvUMfOiy45zeaACYygTHB BUJIZKdQtpms9N/k+HSRqHqJk3CjnrsdQJiDIdPyYA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehjlhhtohgslhgvrhesghhmrghilhdrtghomhdprhgtph htthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepghhithhs thgvrhesphhosghogidrtghomhdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmh grihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:41 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 457c794e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:34 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:39 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 5/9] reftable/stack: simplify tracking of table locks Message-ID: <812a45f3d2ec0709801f95a1eab1ad4b31fc20bc.1723123606.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 Thu Aug 8 14:06:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757514 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 D57CC148307 for ; Thu, 8 Aug 2024 14:06:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126010; cv=none; b=ILjfmYsXkmWsNqRDi7MPk/HkyXRx16f6757oH8Cn1yzKl5lPPiL+M5vHCwWT/FNOwI9XB6WYAZNl56XYSj1eP3QcaHGkB+nS1LTSMVCrNhjQflovYYelcMlezHFR84IHyMIjNpYzEscZEqSmUM8JwC/Hvv/M4z4W4w69ZdhM2IE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126010; c=relaxed/simple; bh=hhmvRLsvNZdvLn+8yJThx4CuP5laQ024gXI7O6AvciA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VSVIBtg1wishuuV2ZP2CcFqchiPpPmb0E2etMMPoX0OZfQn7j9tBkbkmti4nO9CgRyNf4kfRzkNVkshvH/VIxZ/zoTMi3w0N8Nv/HqOOvngwNzfhVjGNVuNJCdezEMd30UZz1jyPu4l0H1KqblvhcXoCVO/cZhQ3EZjsTlTnjmg= 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=iBVBlytg; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=o7DgSdt9; arc=none smtp.client-ip=103.168.172.155 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="iBVBlytg"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="o7DgSdt9" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfhigh.nyi.internal (Postfix) with ESMTP id DAFBD115184B; Thu, 8 Aug 2024 10:06:47 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 08 Aug 2024 10:06:47 -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=1723126007; x=1723212407; bh=jgT4oE975B Byomd8SmQaN2zgz7UwtLH89YKkFjHDpK4=; b=iBVBlytgsNIMt82Z4QcnQ2k19i MXLrMSrbOsg9ZNZBMJMkTa8OIHRxdlxdZe7MRHyCRU6jB3L5pQS+sw8RkTB/P3Sn ij8jmVUiwImp3IWeENlH0gRB23LU+YcAYKfjCUMe/kkrj0lGMz5nQBb5rjceWCar VjEWm3jt4BnsOEWDY1GKGpWBqDY2IRGtb3Fzfybv3J+/CKHjXA10PYD2laAJeA8G W06lJ2SR34PliNs02LdCab8/kjsoxX0SgqAIOap71kBtq1gnEw90556HLIoiuIKx IdSNt5ix8rHjZspjwBwgqp0SO4if2v6eWkctvg6PSFyLf0fwV0PxXbcPdrmQ== 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=1723126007; x=1723212407; bh=jgT4oE975BByomd8SmQaN2zgz7Uw tLH89YKkFjHDpK4=; b=o7DgSdt9XXTmUTXSoDvLNpIk5xR2yGU7gu3PqRlH22oB dJsuWgnU2DbZU8/N0L39pG7ZvQJA0Bei6Q9UVRc8e7Ve3aJyqm6CnOrxJ/o8A1sh ygsCI0NyYDionoLdmD7GjZGjvTYBUBE6wWMEIjflvUPbHVnAltu+W5pwG/XsWbQu vZ9mOcN2JVZbnR1X9aJStjxhGGlMTy3wg7ps7N3ZS0dMSUh6sf0vo/Cnj+n+oC/N 8ddZ4S0lFVTcdq9sz7hzmdKcE+RP4Ii1g4XwNjbYKNr4Hf5xvKL4LR/BxmIpTm6h wNpvRXpzqfMP/4KPwVktMmCteXW/V/7T8udA1QmbLA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpd hrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihht sehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepjhhlthhosghlvghrsehgmh grihhlrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:46 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 7e628f56 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:39 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:44 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 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 Thu Aug 8 14:06:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757515 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 14097148307 for ; Thu, 8 Aug 2024 14:06:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126014; cv=none; b=X60rcroge+r82OxGUIAG2ucyymJ9e5Z9NZwpEol6f9wsygr3h0Sj2iinry0rRfrRm0NDRY0VZJjX87tf87D4Ivr84XFhm1DGCPD4zrVg0j8LanJqouIY0H6wF+nalKl0rEBGwSEZ3eJzFrjZwmB/Djaq+7xF2Uy3FxviyZQh3Cc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126014; c=relaxed/simple; bh=t4yAJCX+7IgPbw8zA5TXYoyHUrkDDHWQYBoBpEAHyKE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kvJtG/xBIO+OcM0IvvxRfROWFLO8JX5U28Eay7pzZTpTSSxoY89CTIhMbkEAdXeT6pqO66uUfBzZPJPzosxq2WlvmozBwTnExvAx9K53vE6pmcr5ffmGpLlgouYGkC2ZZYlKlw5CYuY8hdGE3pFPnk4jWzDYY5hLMzhYKrpLR5Q= 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=DnTPIh5y; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=rp+HHL3z; arc=none smtp.client-ip=103.168.172.155 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="DnTPIh5y"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="rp+HHL3z" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 2E65411516C8; Thu, 8 Aug 2024 10:06:52 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 08 Aug 2024 10:06:52 -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=1723126012; x=1723212412; bh=/VNKo+zVFR BiaLdr9WUcsmyqqfXbhZljSe8XhVaIKG4=; b=DnTPIh5yqMah9hC/IlsdWACEOg AP3h/HFwp63gb+dFJjZiFfuJCH1kgUjeRMKYEALN2hMkK8AkMAZDl2GiKRYO8kAQ 1mpl6+pufcarWaYuQstrQ1kd8Yi9WovkMCjJrXFD2Aq4bUvBmbY9AMusmmZ/4a6s Ny5itFqdAxrcvzrVQ1C3FB9k9Ju9xXAKwvTh2JiIEkeq/qCwOkt+1ObtIWP1GPsL wUorKzD7hWuh8r4QnJpc8X8a46aECkneNg39q5lS43LqEcU5kkIhYLoAF5fzhYAI dF/lqP0E5uVmlmWHXOvA48cfXOFXYJusHLcsY0n2FI8iN4D7dbtvI2n+JrNw== 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=1723126012; x=1723212412; bh=/VNKo+zVFRBiaLdr9WUcsmyqqfXb hZljSe8XhVaIKG4=; b=rp+HHL3zc8SEgZ5YvMk7zgZb+H4C28qkCTRHSBalLdkv 00Vodte1qEH9xoHAT3wALyY3DT4R3Pbgr8dgM+aExWOQyb8QZXJZHjdF/y2GBwn2 ko+E/dCpUnf3J18yImSW61xCT7ejo9AHYY7bX+y8cwGeQguke24TRvCOd4sZqzle IULLRS92sUUV1ukal1PgWkbTFewMTnUSNnNac4f7ObRm6roJWAJLHl6CE1McvxgL VzgvEUAGXwGvIms5Jv/hv+QstgvHCocFjVP4ZoblPdJGvFu/YYE/pNlmiwoR4L1I FWiQl2YT9U2jB0i60KS4mSWQCeGG9B4i13GN8KzW9A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehjlhhtohgslhgvrhesghhmrghilhdrtghomhdprhgtph htthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepkhgrrhht hhhikhdrudekkeesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtshhtvghrsehpoh gsohigrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:51 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c7076626 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:44 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:49 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 7/9] reftable/stack: use lock_file when adding table to "tables.list" Message-ID: <37a757649a621ec193e27ef2939f1d79aaba818d.1723123606.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 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 Thu Aug 8 14:06:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757516 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 22AB563D for ; Thu, 8 Aug 2024 14:06:57 +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=1723126019; cv=none; b=MVuCveGzNNcj+yUA65VvVL1d5X6tWsNG6BNLQsqhQrDPJ+Mcnv3OgDMdzpzb0FC0vH3erDlvC8PW9QMD9tf0o+oLMbeKmRqvsEc7zMoPCObmUqIaWzrHV4huYodO/hHudVsIs/Fsj8l5HkN7UsYF1ULJ3IzhmoprP6hWH3/mXMQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126019; c=relaxed/simple; bh=QyMxiqeVkL3q0K+VCAhGV6KCiUitrwaHLBp2mDrt/zU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DDC5uxalXxfh1uQLILSq+c4AqL6JX5AClmmEqKXbRdClH7hnwfT5PkWK/xfRSjv2eSdrUoh6D/PLXZB1cxczsw5bhbXOtFZ/6Vd2gxVuQOqQhWDsM+6RyFfXkvCTaO4nYLYhLsiozdH9DDObSB5Q4iLzJFbfV0C9nMdZ0Xvts74= 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=I8aq9JUH; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=vBaaVwB8; 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="I8aq9JUH"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="vBaaVwB8" Received: from compute8.internal (compute8.nyi.internal [10.202.2.227]) by mailfout.nyi.internal (Postfix) with ESMTP id 64186138FC9F; Thu, 8 Aug 2024 10:06:57 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute8.internal (MEProxy); Thu, 08 Aug 2024 10:06:57 -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=1723126017; x=1723212417; bh=uovY81k2C6 fziKb3NoOv8QMwyxmAElagFGPCtHAZ0n0=; b=I8aq9JUH3zvnlcmgpIZ/udNBFt t29y9aFvmMucRqyufo9Rj2Nt3O7LJpVDh1/f073w2ortCwcNso0cSyNpf55eTm0w mwVMiQqDj5/lFBJQPh1SFE6WmCWVS0Ns2CQLPRBaPr4dYYVbmJuDzIHGZVGK29hc fEz2d4hEDZAhPaL8U7B7suwFPb6C8crwqfQUo2id06+ExVDZZwScQchwUnosnqOG o4NExTaah/tVkpY0+0/WOf+ymECPfSlKbqBs3PM0n41CpHqmNVRFijCxFpxsKTnx iGtTDkAXhynhLUTbaSpM/kRfQdtNJoLVC68veiG/cB7zi63u8exhJ7IxXD3w== 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=1723126017; x=1723212417; bh=uovY81k2C6fziKb3NoOv8QMwyxmA ElagFGPCtHAZ0n0=; b=vBaaVwB81pzYwTC5WxzW/zYwiinZjyJ+0f5+o5RKmtN9 BVePMkL1Ov9ZqvAaoS2vqgVLgnLX/pZERoOWY4pfkE/LmQD2X76kiFcjI0X748Y0 BTPWCfp6FqnzHjToSXobou53Y669Doqc8pxspJTY0CsyR0Vi4GHjCns2kYokUVRD z2rmSpo4AnFcsBClmAHKesd3/Wgt6Qa8bRNPhgxuuVeEu/000rbJvhAZYiOFZEnq Zyrg+Vj+eaz+I+pGFX0yw/O2ZtXtFXrFK9IBoVlloOQqx9Z23m8EPv+ZUK5jcLRN L0Otjijj1523npkbqTulXDGKElHAmXL9yHjuSHL2vw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpd hrtghpthhtohepjhhlthhosghlvghrsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhi thhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrh hnvghlrdhorhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:56 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 608fbc7a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:49 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:53 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 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..3f13c3eb34 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. + */ + first_to_replace = new_offset; + last_to_replace = last + (new_offset - first); + } 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); + first_to_replace = first; + last_to_replace = last; + } + /* * 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 Thu Aug 8 14:06:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757517 Received: from fhigh4-smtp.messagingengine.com (fhigh4-smtp.messagingengine.com [103.168.172.155]) (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 37B6418E74B for ; Thu, 8 Aug 2024 14:07:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126025; cv=none; b=pGuZRvaRdY5LoI7fih00ksCumxXHA5eki1bIBNX0E6fY/5BgQzlyQQT7RAOBr6Hy0MA7akx0y+psTNTV0dsXm3zyX+Wl0AdntKZSNCI8yLgTPp0uMal5Pj6padHYLQcMyUx58CL48sJCmzskUVYlNA0gV5EYsaNKvCPCWWO0VlY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723126025; c=relaxed/simple; bh=y19g2xY97qY+X+R6tOwBd01wKKoRnEE0eqdCS4e7OSI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Wxz55EveXEDM6BsbVUzD1TZl37uScPlhDJhWodlML+bo15iZ91ECSD1GYlV2mH3n/MS0xuQdTECBOYc3y9CsagT0iSAxgaVYEit8svr179OuRh2nZmghcjcQ98RctwA0iJsD4ozUaH3PnZiXTkE/I0sPa2dWySxQRhzqNW/3xBo= 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=j2bLVG/X; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Gzuj0KJI; arc=none smtp.client-ip=103.168.172.155 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="j2bLVG/X"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Gzuj0KJI" Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 9BB771151025; Thu, 8 Aug 2024 10:07:02 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 08 Aug 2024 10:07:02 -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=1723126022; x=1723212422; bh=c2emqa9Ij4 q4ZqD5TP0N4Jbwh2OQavWjZHO6P8JcUP4=; b=j2bLVG/XQMmw/wr1pNII+OUm7Y zsEJWCgKKvLnUMkNfUnQbOhClw2dqGDr+Da2iJlx085F+bpH00r1EKaq5kR4rgq/ CHRdv2JxZ0XG8OOVtUmsu9849+OgzS/G0eLfzYrDplZt4AxhB36ti+SWngffauYj BTzEj/gP3dOa4Z3TzyuqU8VgUoGS567StTMJQZAPoympzTkjZT0yb5Tx4GpsHIo9 /nydrDKhh4u81SE/92YPR7igE/p77mTCjKC7fnBQikpLaWNGioltl4MPSV68aikj Mj63dLq/9L9NIZwUlONhDbdObxg3Aa9f60E58WwCUFxdzSjgd2miokYjNAPQ== 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=1723126022; x=1723212422; bh=c2emqa9Ij4q4ZqD5TP0N4Jbwh2OQ avWjZHO6P8JcUP4=; b=Gzuj0KJIae6aSNu/S9lB1j0WtoAqI8K5cda4+tM1qh2O T6YP5ERVlNI4yi3/nFpwTEHLVMX43iEG47TfE3v1EFnopm3xpfOlfOn1Tf9bSWhZ i9VxlivONPERmVlvFU2ih9GYVPLye7B0ExXudk+YaXiiTNz/XszcBc92+CmU36kR M1/v2yyHCukDUvMBrRmP9vc2fYf8BSBzFkYuZKPz7D9vSqHQit8tlxaqr1pgcj6v yCQpNg6yBA6QSZtmGUWhD4Ll00yztbS0TQHpBh1RA7MV0CjCFI8KZOsrh2Rj+fnO ESk/Ip1rKRkXEiTUEDfTczJ3EeE1VV0II50AXC9h3w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpeefnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehjlhhtohgslhgvrhesghhmrghilhdrtghomhdprhgtph htthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepkhgrrhht hhhikhdrudekkeesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtshhtvghrsehpoh gsohigrdgtohhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:07:01 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id f343e1df (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:53 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:58 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 9/9] 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 succeeded 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 3f13c3eb34..2071e428a8 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 3ed8e44924..8c36590ff0 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -917,13 +917,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 )