git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Justin Tobler <jltobler@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Karthik Nayak <karthik.188@gmail.com>
Subject: [PATCH v3 5/9] reftable/stack: simplify tracking of table locks
Date: Thu, 8 Aug 2024 16:06:39 +0200	[thread overview]
Message-ID: <812a45f3d2ec0709801f95a1eab1ad4b31fc20bc.1723123606.git.ps@pks.im> (raw)
In-Reply-To: <cover.1723123606.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 3056 bytes --]

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 <ps@pks.im>
---
 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);
-- 
2.46.0.46.g406f326d27.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-08-08 14:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 14:14 [PATCH 0/8] reftable: improvements and fixes for compaction Patrick Steinhardt
2024-07-31 14:14 ` [PATCH 1/8] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-02 20:26   ` Junio C Hamano
2024-07-31 14:14 ` [PATCH 2/8] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-02 21:05   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 3/8] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 4/8] reftable/stack: simplify tracking of table locks Patrick Steinhardt
2024-07-31 21:57   ` Justin Tobler
2024-08-02 23:00   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 5/8] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 6/8] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-07-31 23:02   ` Justin Tobler
2024-08-01  8:40     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 7/8] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-01  1:04   ` Justin Tobler
2024-08-01  8:41     ` Patrick Steinhardt
2024-07-31 14:15 ` [PATCH 8/8] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-02 23:24   ` Junio C Hamano
2024-08-05 12:11     ` Patrick Steinhardt
2024-08-05 13:07 ` [PATCH v2 0/9] reftable: improvements and fixes for compaction Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 1/9] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 2/9] reftable/stack: extract function to setup stack with N tables Patrick Steinhardt
2024-08-05 13:07   ` [PATCH v2 3/9] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-08 10:46     ` Karthik Nayak
2024-08-05 13:08   ` [PATCH v2 4/9] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 5/9] reftable/stack: simplify tracking of table locks Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 6/9] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 7/9] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 8/9] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-08 12:14     ` Karthik Nayak
2024-08-08 13:29       ` Patrick Steinhardt
2024-08-05 13:08   ` [PATCH v2 9/9] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-06 18:46     ` Justin Tobler
2024-08-07  5:31       ` Patrick Steinhardt
2024-08-07 19:12         ` Justin Tobler
2024-08-08 12:25     ` Karthik Nayak
2024-08-08 12:26   ` [PATCH v2 0/9] reftable: improvements and fixes for compaction Karthik Nayak
2024-08-08 14:06 ` [PATCH v3 " Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 1/9] reftable/stack: refactor function to gather table sizes Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 2/9] reftable/stack: extract function to setup stack with N tables Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 3/9] reftable/stack: test compaction with already-locked tables Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 4/9] reftable/stack: update stats on failed full compaction Patrick Steinhardt
2024-08-08 14:06   ` Patrick Steinhardt [this message]
2024-08-08 14:06   ` [PATCH v3 6/9] reftable/stack: do not die when fsyncing lock file files Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 7/9] reftable/stack: use lock_file when adding table to "tables.list" Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 8/9] reftable/stack: fix corruption on concurrent compaction Patrick Steinhardt
2024-08-08 14:06   ` [PATCH v3 9/9] reftable/stack: handle locked tables during auto-compaction Patrick Steinhardt
2024-08-08 16:52   ` [PATCH v3 0/9] reftable: improvements and fixes for compaction Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=812a45f3d2ec0709801f95a1eab1ad4b31fc20bc.1723123606.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).