Re: [PATCH v4 2/6] core.fsyncobjectfiles: batched disk flushes

On Tue, Sep 21, 2021 at 6:23 PM Neeraj Singh [off-list ref] wrote:

On Tue, Sep 21, 2021 at 4:41 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

On Mon, Sep 20 2021, Neeraj Singh via GitGitGadget wrote:

quoted

quoted

-             fsync_object_files = git_config_bool(var, value);
+             if (value && !strcmp(value, "batch"))
+                     fsync_object_files = FSYNC_OBJECT_FILES_BATCH;
+             else if (git_config_bool(var, value))
+                     fsync_object_files = FSYNC_OBJECT_FILES_ON;
+             else
+                     fsync_object_files = FSYNC_OBJECT_FILES_OFF;

Since the point of this setting is safety, let's explicitly check
true/false here, use git_config_maybe_bool(), and perhaps issue a
warning on unknown values, but maybe that would get too verbose...

If we have a future "supersafe" mode, it'll get mapped to "false" on
older versions of git, probably not a good idea...

 I took Junio's suggestion verbatim.  I'll try a warning if the value
exists, and is not 'batch' or <maybe bool>.

An update on this.  I tested out some values:
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=batch add ./
    fsync_object_files: 2
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=0 add ./
    fsync_object_files: 0
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=1 add ./
    fsync_object_files: 1
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=2 add ./
    fsync_object_files: 1
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=barf add ./
    fatal: bad boolean config value 'barf' for 'core.fsyncobjectfiles'
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=true add ./
    fsync_object_files: 1
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=false add ./
    fsync_object_files: 0
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=t add ./
    fatal: bad boolean config value 't' for 'core.fsyncobjectfiles'
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=y add ./
    fatal: bad boolean config value 'y' for 'core.fsyncobjectfiles'
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=yes add ./
    fsync_object_files: 1
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=no add ./
    fsync_object_files: 0
    nksingh@neerajsi-x1:~/src/git$ ./git -c core.fsyncobjectfiles=nope add ./
    fatal: bad boolean config value 'nope' for 'core.fsyncobjectfiles'

So I think the code already works like you are suggesting (thanks Junio!).