All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: John Cai via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, John Cai <johncai86@gmail.com>
Subject: Re: [PATCH] reftable: honor core.fsync
Date: Mon, 29 Jan 2024 10:48:59 +0100	[thread overview]
Message-ID: <Zbd0i9nOeWWNQ2EW@tanuki> (raw)
In-Reply-To: <pull.1654.git.git.1706035870956.gitgitgadget@gmail.com>

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

On Tue, Jan 23, 2024 at 06:51:10PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> While the reffiles backend honors configured fsync settings, the
> reftable backend does not. Address this by fsyncing reftable files using
> the write-or-die api's fsync_component() in two places: when we
> add additional entries into the table, and when we close the reftable
> writer.
> 
> This commits adds a flush function pointer as a new member of
> reftable_writer because we are not sure that the first argument to the
> *write function pointer always contains a file descriptor. In the case of
> strbuf_add_void, the first argument is a buffer. This way, we can pass
> in a corresponding flush function that knows how to flush depending on
> which writer is being used.
> 
> This patch does not contain tests as they will need to wait for another
> patch to start to exercise the reftable backend. At that point, the
> tests will be added to observe that fsyncs are happening when the
> reftable is in use.
> 
> Signed-off-by: John Cai <johncai86@gmail.com>

I noticed that we missed syncing the "tables.list" file when performing
auto-compaction. The below patch is needed on top of what we already
have.

The topic is currently in `next`, but not yet in `master`, so we might
still squash it in. Junio, please let me know whether you want to do so
or whether I shall send this fix-up as a new patch. Thanks!

Patrick

diff --git a/reftable/stack.c b/reftable/stack.c
index ab295341cc..b17cfb9516 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1018,6 +1018,10 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
 		unlink(new_table_path.buf);
 		goto done;
 	}
+
+	fsync_component_or_die(FSYNC_COMPONENT_REFERENCE, lock_file_fd,
+			       lock_file_name.buf);
+
 	err = close(lock_file_fd);
 	lock_file_fd = -1;
 	if (err < 0) {

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

  parent reply	other threads:[~2024-01-29  9:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-23 18:51 [PATCH] reftable: honor core.fsync John Cai via GitGitGadget
2024-01-23 19:31 ` Junio C Hamano
2024-01-23 21:42   ` John Cai
2024-01-23 21:50   ` Junio C Hamano
2024-01-24  8:41     ` Patrick Steinhardt
2024-01-24 17:22       ` Junio C Hamano
2024-01-23 21:06 ` Kristoffer Haugsbakk
2024-01-23 21:38   ` John Cai
2024-01-29  9:48 ` Patrick Steinhardt [this message]
2024-01-29 17:15   ` Junio C Hamano

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=Zbd0i9nOeWWNQ2EW@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johncai86@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 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.