All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Justin Tobler <jltobler@gmail.com>, Toon claes <toon@iotcl.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 4/4] reftable/stack: register compacted tables as tempfiles
Date: Thu, 7 Mar 2024 14:10:43 +0100	[thread overview]
Message-ID: <4023d78f084c8df7ffd6c5ed68b8d0eadc713fd0.1709816483.git.ps@pks.im> (raw)
In-Reply-To: <cover.1709816483.git.ps@pks.im>

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

We do not register tables resulting from stack compaction with the
tempfile API. Those tables will thus not be deleted in case Git gets
killed.

Refactor the code to register compacted tables as tempfiles.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 54 +++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index 977336b7d5..1ecf1b9751 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -827,51 +827,56 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st)
 
 static int stack_compact_locked(struct reftable_stack *st,
 				size_t first, size_t last,
-				struct strbuf *temp_tab,
-				struct reftable_log_expiry_config *config)
+				struct reftable_log_expiry_config *config,
+				struct tempfile **tab_file_out)
 {
 	struct strbuf next_name = STRBUF_INIT;
-	int tab_fd = -1;
+	struct strbuf tab_file_path = STRBUF_INIT;
 	struct reftable_writer *wr = NULL;
-	int err = 0;
+	struct tempfile *tab_file;
+	int tab_fd, err = 0;
 
 	format_name(&next_name,
 		    reftable_reader_min_update_index(st->readers[first]),
 		    reftable_reader_max_update_index(st->readers[last]));
+	stack_filename(&tab_file_path, st, next_name.buf);
+	strbuf_addstr(&tab_file_path, ".temp.XXXXXX");
 
-	stack_filename(temp_tab, st, next_name.buf);
-	strbuf_addstr(temp_tab, ".temp.XXXXXX");
+	tab_file = mks_tempfile(tab_file_path.buf);
+	if (!tab_file) {
+		err = REFTABLE_IO_ERROR;
+		goto done;
+	}
+	tab_fd = get_tempfile_fd(tab_file);
 
-	tab_fd = mkstemp(temp_tab->buf);
 	if (st->config.default_permissions &&
-	    chmod(temp_tab->buf, st->config.default_permissions) < 0) {
+	    chmod(get_tempfile_path(tab_file), st->config.default_permissions) < 0) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
 
-	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &st->config);
-
+	wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush,
+				 &tab_fd, &st->config);
 	err = stack_write_compact(st, wr, first, last, config);
 	if (err < 0)
 		goto done;
+
 	err = reftable_writer_close(wr);
 	if (err < 0)
 		goto done;
 
-	err = close(tab_fd);
-	tab_fd = 0;
+	err = close_tempfile_gently(tab_file);
+	if (err < 0)
+		goto done;
+
+	*tab_file_out = tab_file;
+	tab_file = NULL;
 
 done:
+	delete_tempfile(&tab_file);
 	reftable_writer_free(wr);
-	if (tab_fd > 0) {
-		close(tab_fd);
-		tab_fd = 0;
-	}
-	if (err != 0 && temp_tab->len > 0) {
-		unlink(temp_tab->buf);
-		strbuf_release(temp_tab);
-	}
 	strbuf_release(&next_name);
+	strbuf_release(&tab_file_path);
 	return err;
 }
 
@@ -979,12 +984,12 @@ static int stack_compact_range(struct reftable_stack *st,
 			       struct reftable_log_expiry_config *expiry)
 {
 	struct strbuf tables_list_buf = STRBUF_INIT;
-	struct strbuf new_table_temp_path = STRBUF_INIT;
 	struct strbuf new_table_name = STRBUF_INIT;
 	struct strbuf new_table_path = STRBUF_INIT;
 	struct strbuf table_name = STRBUF_INIT;
 	struct lock_file tables_list_lock = LOCK_INIT;
 	struct lock_file *table_locks = NULL;
+	struct tempfile *new_table = NULL;
 	int is_empty_table = 0, err = 0;
 	size_t i;
 
@@ -1059,7 +1064,7 @@ static int stack_compact_range(struct reftable_stack *st,
 	 * these tables may end up with an empty new table in case tombstones
 	 * end up cancelling out all refs in that range.
 	 */
-	err = stack_compact_locked(st, first, last, &new_table_temp_path, expiry);
+	err = stack_compact_locked(st, first, last, expiry, &new_table);
 	if (err < 0) {
 		if (err != REFTABLE_EMPTY_TABLE_ERROR)
 			goto done;
@@ -1099,7 +1104,7 @@ static int stack_compact_range(struct reftable_stack *st,
 		strbuf_addstr(&new_table_name, ".ref");
 		stack_filename(&new_table_path, st, new_table_name.buf);
 
-		err = rename(new_table_temp_path.buf, new_table_path.buf);
+		err = rename_tempfile(&new_table, new_table_path.buf);
 		if (err < 0) {
 			err = REFTABLE_IO_ERROR;
 			goto done;
@@ -1166,9 +1171,10 @@ static int stack_compact_range(struct reftable_stack *st,
 		rollback_lock_file(&table_locks[i - first]);
 	reftable_free(table_locks);
 
+	delete_tempfile(&new_table);
 	strbuf_release(&new_table_name);
 	strbuf_release(&new_table_path);
-	strbuf_release(&new_table_temp_path);
+
 	strbuf_release(&tables_list_buf);
 	strbuf_release(&table_name);
 	return err;
-- 
2.44.0


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

      parent reply	other threads:[~2024-03-07 13:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04 11:10 [PATCH 0/4] reftable/stack: register temporary files Patrick Steinhardt
2024-03-04 11:10 ` [PATCH 1/4] lockfile: report when rollback fails Patrick Steinhardt
2024-03-05 22:09   ` Justin Tobler
2024-03-06 12:00     ` Patrick Steinhardt
2024-03-04 11:10 ` [PATCH 2/4] reftable/stack: register new tables as tempfiles Patrick Steinhardt
2024-03-05 22:30   ` Justin Tobler
2024-03-06 11:59     ` Patrick Steinhardt
2024-03-06 16:34       ` Justin Tobler
2024-03-06 16:36       ` Junio C Hamano
2024-03-07  6:17         ` Patrick Steinhardt
2024-03-07 17:59           ` Junio C Hamano
2024-03-07 20:54             ` Patrick Steinhardt
2024-03-07 21:06               ` Junio C Hamano
2024-03-04 11:10 ` [PATCH 3/4] reftable/stack: register lockfiles during compaction Patrick Steinhardt
2024-03-05 23:30   ` Justin Tobler
2024-03-06 11:59     ` Patrick Steinhardt
2024-03-06 16:39       ` Junio C Hamano
2024-03-06 19:57       ` Justin Tobler
2024-03-04 11:10 ` [PATCH 4/4] reftable/stack: register compacted tables as tempfiles Patrick Steinhardt
2024-03-07 12:38   ` Toon claes
2024-03-07 12:58     ` Patrick Steinhardt
2024-03-07 13:10 ` [PATCH v2 0/4] reftable/stack: register temporary files Patrick Steinhardt
2024-03-07 13:10   ` [PATCH v2 1/4] lockfile: report when rollback fails Patrick Steinhardt
2024-03-07 13:10   ` [PATCH v2 2/4] reftable/stack: register new tables as tempfiles Patrick Steinhardt
2024-03-07 13:10   ` [PATCH v2 3/4] reftable/stack: register lockfiles during compaction Patrick Steinhardt
2024-03-07 13:10   ` Patrick Steinhardt [this message]

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=4023d78f084c8df7ffd6c5ed68b8d0eadc713fd0.1709816483.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=toon@iotcl.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.