Re: [PATCH v2 2/7] core.fsyncmethod: batched disk flushes for loose-objects

From: Neeraj Singh <nksingh85@gmail.com>
Date: Mar 21, 2022, 18:28 UTC
Message-ID: <CANQDOdcPAb31JY6LdMiCn26192_1wvKkHodpNEBkSh3WbD+e4g@mail.gmail.com>
In-Reply-To: <220321.86a6dj9xja.gmgdl@evledraar.gmail.com>

On Mon, Mar 21, 2022 at 7:43 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 14 quoted lines

>
>
> On Sun, Mar 20 2022, Neeraj Singh via GitGitGadget wrote:
>
> > From: Neeraj Singh <neerajsi@microsoft.com>
> > [...]
> > +     if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT)) {
> > +             bulk_fsync_objdir = tmp_objdir_create("bulk-fsync");
> > +             if (!bulk_fsync_objdir)
> > +                     die(_("Could not create temporary object directory for core.fsyncobjectfiles=batch"));
>
> Should camel-case the config var, and we should have a die_errno() here
> which tell us why we couldn't create it (possibly needing to ferry it up
> from the tempfile API...)

Thanks for noticing the camelCasing. The config var name was also wrong. Now it will read:

> > +                     die(_("Could not create temporary object directory for core.fsyncMethod=batch"));

Do you have any recommendations on how to easily ferry the correct errno out of tmp_objdir_create? It looks like the remerge-diff usage has the same die behavior w/o errno, and the builtin/receive-pack.c usage doesn't die, but also loses the errno. I'm concerned about preserving the errno across the or tmp_objdir_destroy calls. I could introduce a temp errno var to preserve it across those. Is that what you had in mind?

Thanks, Neeraj