From patchwork Thu Aug 8 14:06:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13757508 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 84B9018E052 for ; Thu, 8 Aug 2024 14:06:20 +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=1723125982; cv=none; b=uccBf8r8YIP+SVYNEv1XMMpr5+asFPxc9bk0Ij3mZlaEaK4sOevgJ76QlksMV+pWwqbdOjuyEO4IJgJFQzs2pbSKCvHenYu2ou3vxa825buxs7K19tBqOlOuBSYwn6XDAgJQU1io3n4T5UYE4WPHmrytQnLwXFmxgiYZW3K2+C4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723125982; c=relaxed/simple; bh=zSRmWXz8YtNmuUQPh+UfLweRj2dsGhMKqjk6U+SQoCM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bmCRTqkOjWV27OLgC+QtyA0glYu5PEk8KkBxRdOD4SEBvTwUzt3ocQEFBsz2a+iQHjj7s989e48wFKN6W4vMTVqU4xoMgarpGyj23n9JpiaXefZUOFlTv8veHYJmj94mP0mE0NzVyj2vxEZduHcTNAFXpksw+htc0jpHRaI8IhA= 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=RFK7oSH/; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=spDzGRe5; 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="RFK7oSH/"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="spDzGRe5" Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 7DBA91150DE1; Thu, 8 Aug 2024 10:06:19 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Thu, 08 Aug 2024 10:06:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1723125979; x=1723212379; bh=Ef/0DA7jNJ voWwu8f4gJcGRb7IB2YrBZU1YZ92uLPLM=; b=RFK7oSH/Gmzb5MfBzohLcprscx hChsjgS5V6TIMOYgoWo7jIrBEQcCpTByZ7X+DzaBnQZodpHynRZZUk/aZcKJFkuv ZhRSCT1LxNTbcEb9ZvpVROuttj3qDyEceGjBmFSbZb2O1dvp/cy6JOV5E37XxqAr yfdRyV+Tg7zlPPWW1vb4kaV5qtHNJGHZIx4Y8Pw61wNn4jDk3swDomnfXQ4LjbcE KSpO+hJkV3GJKiuHxGMK39Grab9KveJVfPYLjVYBcRRBHyR35jX0IZlca2NPIN7f ue+nZk1yCb05rSKSUOrMyjsJW8l+2mEWC+mgmqm+/mTRNeAmnYi5PXP/fUjQ== 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=1723125979; x=1723212379; bh=Ef/0DA7jNJvoWwu8f4gJcGRb7IB2 YrBZU1YZ92uLPLM=; b=spDzGRe507FfsdBelA+LzVoBQ5m5VKcACgfb1/e67swm EEj0LTT9jPuiUPmE4yJyV8FY+u1a106g8SO3Z28RaU0O+myNYoQ+HMeXVJp3+3rp wTCJGoCxkRGYKQZGGPK1nlItB1/NAJTPlF/0KYQ3OMCfGPG1ldKCOZgpDCRjRWWM PTOCAk9fgc16SOx1MQPGd2O6IDu8Ng659/qbYjUUG9qZDuf31+mlvueqH7dECrmT gqEC67cvkmxCI6EJ4338mNaZX6vqcDMyASgNn6lFn2CC9hFZQEP0u1kjnTe5sMAE f0rB9T4i6iIYSvYS+ohPeAho2GbNhzUFQYdQ9tVG4g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrledvgdejvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecu hfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqe enucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegkeeluddvfeefgeeh gfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeegpdhmohguvgepshhm thhpohhuthdprhgtphhtthhopehjlhhtohgslhgvrhesghhmrghilhdrtghomhdprhgtph htthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhi thhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrh hnvghlrdhorhhg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 8 Aug 2024 10:06:18 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c66f223a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 8 Aug 2024 14:06:09 +0000 (UTC) Date: Thu, 8 Aug 2024 16:06:14 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Justin Tobler , Junio C Hamano , Karthik Nayak Subject: [PATCH v3 0/9] reftable: improvements and fixes for 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: Hi, this is the second version of my patch series that aims to improve the way reftable stack perform compaction. Changes compared to v2: - Drop the unused `reftable_write_options` structure in `write_n_ref_tables()`. - Fix a commit message typo. - Reorder some variable assignments to feel more natural. Thanks! Patrick Patrick Steinhardt (9): reftable/stack: refactor function to gather table sizes reftable/stack: extract function to setup stack with N tables reftable/stack: test compaction with already-locked tables reftable/stack: update stats on failed full compaction reftable/stack: simplify tracking of table locks reftable/stack: do not die when fsyncing lock file files reftable/stack: use lock_file when adding table to "tables.list" reftable/stack: fix corruption on concurrent compaction reftable/stack: handle locked tables during auto-compaction reftable/stack.c | 231 +++++++++++++++++++++++++++++-------- reftable/stack_test.c | 142 ++++++++++++++++++----- t/t0610-reftable-basics.sh | 21 ++-- 3 files changed, 310 insertions(+), 84 deletions(-) Range-diff against v2: 1: 6d2b54ba8e = 1: 6d2b54ba8e reftable/stack: refactor function to gather table sizes 2: ff17306cc0 ! 2: 798a661824 reftable/stack: extract function to setup stack with N tables @@ Commit message 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 ## @@ reftable/stack_test.c: static int write_test_ref(struct reftable_writer *wr, voi } +static void write_n_ref_tables(struct reftable_stack *st, -+ struct reftable_write_options *opts, + 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), @@ reftable/stack_test.c: static int write_test_ref(struct reftable_writer *wr, voi + EXPECT_ERR(err); + } + ++ st->opts.disable_auto_compact = disable_auto_compact; + strbuf_release(&buf); +} + @@ reftable/stack_test.c: static void test_reftable_stack_compaction_concurrent(voi - err = reftable_stack_add(st1, &write_test_ref, &ref); - EXPECT_ERR(err); - } -+ write_n_ref_tables(st1, &opts, 3); ++ write_n_ref_tables(st1, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); @@ reftable/stack_test.c: static void test_reftable_stack_compaction_concurrent_cle - err = reftable_stack_add(st1, &write_test_ref, &ref); - EXPECT_ERR(err); - } -+ write_n_ref_tables(st1, &opts, 3); ++ write_n_ref_tables(st1, 3); err = reftable_new_stack(&st2, dir, &opts); EXPECT_ERR(err); 3: 63e64c8d82 ! 3: 949f748823 reftable/stack: test compaction with already-locked tables @@ reftable/stack_test.c: static void test_reftable_stack_auto_compaction(void) + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + -+ write_n_ref_tables(st, &opts, 5); ++ write_n_ref_tables(st, 5); + EXPECT(st->merged->stack_len == 5); + + /* @@ reftable/stack_test.c: static void test_reftable_stack_add_performs_auto_compact + err = reftable_new_stack(&st, dir, &opts); + EXPECT_ERR(err); + -+ write_n_ref_tables(st, &opts, 3); ++ write_n_ref_tables(st, 3); + EXPECT(st->merged->stack_len == 3); + + /* Lock one of the tables that we're about to compact. */ 4: 1989dafcb4 = 4: 478f945a45 reftable/stack: update stats on failed full compaction 5: 73e5d104eb = 5: 812a45f3d2 reftable/stack: simplify tracking of table locks 6: e411e14904 = 6: d7d34e7253 reftable/stack: do not die when fsyncing lock file files 7: b868a518d6 = 7: 37a757649a reftable/stack: use lock_file when adding table to "tables.list" 8: ff17414d26 ! 8: b27cb325fc reftable/stack: fix corruption on concurrent compaction @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, + * 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; ++ last_to_replace = last + (new_offset - first); + } else { + /* + * `fd_read_lines()` uses a `NULL` sentinel to indicate that @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, + 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; ++ last_to_replace = last; + } + /* 9: 1ef560feb1 ! 9: dc2fea145d reftable/stack: handle locked tables during auto-compaction @@ Commit message 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 + 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.