Skip to content

pr-1093/neerajsi-msft/ns/core-fsync-v6

This is an implementation of an extensible configuration mechanism for
fsyncing persistent components of a repo.

The main goals are to separate the "what" to sync from the "how". There are
now two settings: core.fsync - Control the 'what', including the index.
core.fsyncMethod - Control the 'how'. Currently we support writeout-only and
full fsync.

Syncing of refs can be layered on top of core.fsync. And batch mode will be
layered on core.fsyncMethod. Once this series reaches 'seen', I'll submit
ns/batched-fsync to introduce batch mode. Please see
https://github.com/gitgitgadget/git/pull/1134.

core.fsyncObjectfiles is removed and we will issue a deprecation warning if
it's seen.

I'd like to get agreement on this direction before submitting batch mode to
the list. The batch mode series is available to view at

Please see [1], [2], and [3] for discussions that led to this series.

After this change, new persistent data files added to the repo will need to
be added to the fsync_component enum and documented in the
Documentation/config/core.txt text.

V6 changes:

 * Move only the windows csprng includes into wrapper.c rather than all of
   them. This fixes the specific build issue due to broken Windows headers.
   [6]
 * Split the configuration parsing of core.fsync from the mechanism to focus
   the review.
 * Incorporate Patrick's patch at [7] into the core.fsync mechanism patch.
 * Pick the stricter one of core.fsyncObjectFiles and (fsync_components &
   FSYNC_COMPONENT_LOOSE_OBJECTS), to respect the older setting.
 * Issue a deprecation warning but keep parsing and honoring
   core.fsyncObjectFiles.
 * Change configuration parsing of core.fsync to always start with the
   platform default. none resets to the empty set. The comma separated list
   implies a set without regards to ordering now. This follows Junio's
   suggestion in [8].
 * Change the documentation of the core.fsync option to reflect the way the
   new parsing code works.
 * The patch 7 and 8 of Patrick's series at [7] can be cherry-picked after
   being applied to ns/core-fsyncmethod.

V5 changes:

 * Rebase onto main at c2162907e9
 * Add a patch to move CSPRNG platform includes to wrapper.c. This avoids
   build errors in compat/win32/flush.c and other files.
 * Move the documentation and aggregate options to the final patch in the
   series.
 * Define new aggregate options and guidance in line with Junio's suggestion
   to present the user with 'levels of safety' rather than a morass of
   detailed options.

V4 changes:

 * Rebase onto master at b23dac905bd.
 * Add a comment to write_pack_file indicating why we don't fsync when
   writing to stdout.
 * I kept the configuration schema as-is rather than switching to
   multi-value. The thinking here is that a stateless last-one-wins config
   schema (comma separated) will make it easier to achieve some holistic
   self-consistent fsync configuration for a particular repo.

V3 changes:

 * Remove relative path from git-compat-util.h include [4].
 * Updated newly added warning texts to have more context for localization
   [4].
 * Fixed tab spacing in enum fsync_action
 * Moved the fsync looping out to a helper and do it consistently. [4]
 * Changed commit description to use camelCase for config names. [5]
 * Add an optional fourth patch with derived-metadata so that the user can
   exclude a forward-compatible set of things that should be recomputable
   given existing data.

V2 changes:

 * Updated the documentation for core.fsyncmethod to be less certain.
   writeout-only probably does not do the right thing on Linux.
 * Split out the core.fsync=index change into its own commit.
 * Rename REPO_COMPONENT to FSYNC_COMPONENT. This is really specific to
   fsyncing, so the name should reflect that.
 * Re-add missing Makefile change for SYNC_FILE_RANGE.
 * Tested writeout-only mode, index syncing, and general config settings.

[1] https://lore.kernel.org/git/211110.86r1bogg27.gmgdl@evledraar.gmail.com/
[2]
https://lore.kernel.org/git/dd65718814011eb93ccc4428f9882e0f025224a6.1636029491.git.ps@pks.im/
[3]
https://lore.kernel.org/git/pull.1076.git.git.1629856292.gitgitgadget@gmail.com/
[4]
https://lore.kernel.org/git/CANQDOdf8C4-haK9=Q_J4Cid8bQALnmGDm=SvatRbaVf+tkzqLw@mail.gmail.com/
[5] https://lore.kernel.org/git/211207.861r2opplg.gmgdl@evledraar.gmail.com/
[6]
https://lore.kernel.org/git/CANQDOdfZbOHZQt9Ah0t1AamTO2T7Gq0tmWX1jLqL6njE0LF6DA@mail.gmail.com/
[7]
https://lore.kernel.org/git/50e39f698a7c0cc06d3bc060e6dbc539ea693241.1646905589.git.ps@pks.im/
[8] https://lore.kernel.org/git/xmqqk0d1cxsv.fsf@gitster.g/

Neeraj Singh (6):
  wrapper: make inclusion of Windows csprng header tightly scoped
  core.fsyncmethod: add writeout-only mode
  core.fsync: introduce granular fsync control infrastructure
  core.fsync: add configuration parsing
  core.fsync: new option to harden the index
  core.fsync: documentation and user-friendly aggregate options

 Documentation/config/core.txt       | 58 ++++++++++++++++--
 Makefile                            |  6 ++
 builtin/fast-import.c               |  2 +-
 builtin/index-pack.c                |  4 +-
 builtin/pack-objects.c              | 24 +++++---
 bulk-checkin.c                      |  5 +-
 cache.h                             | 48 +++++++++++++++
 commit-graph.c                      |  3 +-
 compat/mingw.h                      |  3 +
 compat/win32/flush.c                | 28 +++++++++
 compat/winansi.c                    |  5 --
 config.c                            | 94 +++++++++++++++++++++++++++++
 config.mak.uname                    |  3 +
 configure.ac                        |  8 +++
 contrib/buildsystems/CMakeLists.txt | 16 +++--
 csum-file.c                         |  5 +-
 csum-file.h                         |  3 +-
 environment.c                       |  4 +-
 git-compat-util.h                   | 30 +++++++--
 midx.c                              |  3 +-
 object-file.c                       | 13 ++--
 pack-bitmap-write.c                 |  3 +-
 pack-write.c                        | 13 ++--
 read-cache.c                        | 19 ++++--
 wrapper.c                           | 71 ++++++++++++++++++++++
 write-or-die.c                      | 33 ++++++++--
 26 files changed, 444 insertions(+), 60 deletions(-)
 create mode 100644 compat/win32/flush.c

base-commit: c2162907e9aa884bdb70208389cb99b181620d51

Submitted-As: https://lore.kernel.org/git/pull.1093.v6.git.1646952204.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1093.git.1638588503.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1093.v2.git.1638845211.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1093.v3.git.1639011433.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1093.v4.git.1643686424.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1093.v5.git.1646866998.gitgitgadget@gmail.com
Assets 2