diff mbox series

[v7] core.fsync: documentation and user-friendly aggregate options

Message ID 20220315191245.17990-1-neerajsi@microsoft.com (mailing list archive)
State Accepted
Commit b9f5d0358d2e882d47f496c1a5589f6cebc25578
Headers show
Series [v7] core.fsync: documentation and user-friendly aggregate options | expand

Commit Message

Neeraj Singh March 15, 2022, 7:12 p.m. UTC
This commit adds aggregate options for the core.fsync setting that are
more user-friendly. These options are specified in terms of 'levels of
safety', indicating which Git operations are considered to be sync
points for durability.

The new documentation is also included here in its entirety for ease of
review.

Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
---
This revision fixes a grammatical mistake in the core.fsync documentation.

 Documentation/config/core.txt | 40 +++++++++++++++++++++++++++++++++++
 cache.h                       | 23 +++++++++++++++++---
 config.c                      |  5 +++++
 3 files changed, 65 insertions(+), 3 deletions(-)

Comments

Junio C Hamano March 15, 2022, 7:32 p.m. UTC | #1
Neeraj Singh <nksingh85@gmail.com> writes:

> This commit adds aggregate options for the core.fsync setting that are
> more user-friendly. These options are specified in terms of 'levels of
> safety', indicating which Git operations are considered to be sync
> points for durability.
>
> The new documentation is also included here in its entirety for ease of
> review.
>
> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> ---
> This revision fixes a grammatical mistake in the core.fsync documentation.

Is this meant to be [PATCH v7 6/6] where 1-5/6 of v7 are supposed to
be identical to their counterparts of v6 and therefor not sent?  Not
complaining, just double-checking, as I do not want to assume and
end up missing the final updates to the other 5.

In the meantime, I'd assume that it is the case, and will fix the
author ident (you sent this from your gmail address) before
replacing the last bit.  

The only change from the previous round is "the platform default on
most platform" -> "the default on most platforms", which looks
sensible.

Thanks.

1:  39f4b94c2c ! 1:  dfeab99d23 core.fsync: documentation and user-friendly aggregate options
    @@
      ## Metadata ##
    -Author: Neeraj Singh <neerajsi@microsoft.com>
    +Author: Neeraj Singh <nksingh85@gmail.com>
     
      ## Commit message ##
         core.fsync: documentation and user-friendly aggregate options
    @@ Documentation/config/core.txt: core.whitespace::
     +is ignored.
     ++
     +The empty string resets the fsync configuration to the platform
    -+default. The platform default on most platform is equivalent to
    ++default. The default on most platforms is equivalent to
     +`core.fsync=committed,-loose-object`, which has good performance,
     +but risks losing recent work in the event of an unclean system shutdown.
     ++

Thanks.
Neeraj Singh March 15, 2022, 7:56 p.m. UTC | #2
On Tue, Mar 15, 2022 at 12:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Neeraj Singh <nksingh85@gmail.com> writes:
>
> > This commit adds aggregate options for the core.fsync setting that are
> > more user-friendly. These options are specified in terms of 'levels of
> > safety', indicating which Git operations are considered to be sync
> > points for durability.
> >
> > The new documentation is also included here in its entirety for ease of
> > review.
> >
> > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
> > ---
> > This revision fixes a grammatical mistake in the core.fsync documentation.
>
> Is this meant to be [PATCH v7 6/6] where 1-5/6 of v7 are supposed to
> be identical to their counterparts of v6 and therefor not sent?  Not
> complaining, just double-checking, as I do not want to assume and
> end up missing the final updates to the other 5.
>
> In the meantime, I'd assume that it is the case, and will fix the
> author ident (you sent this from your gmail address) before
> replacing the last bit.
>
> The only change from the previous round is "the platform default on
> most platform" -> "the default on most platforms", which looks
> sensible.
>
> Thanks.

Yes, that's right.  I was trying out git-send-email for the first
time.   I didn't want to spam the list with a whole new series where
only one patch had a change.

>
> 1:  39f4b94c2c ! 1:  dfeab99d23 core.fsync: documentation and user-friendly aggregate options
>     @@
>       ## Metadata ##
>     -Author: Neeraj Singh <neerajsi@microsoft.com>
>     +Author: Neeraj Singh <nksingh85@gmail.com>
>
>       ## Commit message ##
>          core.fsync: documentation and user-friendly aggregate options
>     @@ Documentation/config/core.txt: core.whitespace::
>      +is ignored.
>      ++
>      +The empty string resets the fsync configuration to the platform
>     -+default. The platform default on most platform is equivalent to
>     ++default. The default on most platforms is equivalent to
>      +`core.fsync=committed,-loose-object`, which has good performance,
>      +but risks losing recent work in the event of an unclean system shutdown.
>      ++
>
> Thanks.
diff mbox series

Patch

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index ab911d6e26..9a3ad71e9e 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -547,6 +547,46 @@  core.whitespace::
   is relevant for `indent-with-non-tab` and when Git fixes `tab-in-indent`
   errors. The default tab width is 8. Allowed values are 1 to 63.
 
+core.fsync::
+	A comma-separated list of components of the repository that
+	should be hardened via the core.fsyncMethod when created or
+	modified.  You can disable hardening of any component by
+	prefixing it with a '-'.  Items that are not hardened may be
+	lost in the event of an unclean	system shutdown. Unless you
+	have special requirements, it is recommended that you leave
+	this option empty or pick one of `committed`, `added`,
+	or `all`.
++
+When this configuration is encountered, the set of components starts with
+the platform default value, disabled components are removed, and additional
+components are added. `none` resets the state so that the platform default
+is ignored.
++
+The empty string resets the fsync configuration to the platform
+default. The default on most platforms is equivalent to
+`core.fsync=committed,-loose-object`, which has good performance,
+but risks losing recent work in the event of an unclean system shutdown.
++
+* `none` clears the set of fsynced components.
+* `loose-object` hardens objects added to the repo in loose-object form.
+* `pack` hardens objects added to the repo in packfile form.
+* `pack-metadata` hardens packfile bitmaps and indexes.
+* `commit-graph` hardens the commit graph file.
+* `index` hardens the index when it is modified.
+* `objects` is an aggregate option that is equivalent to
+  `loose-object,pack`.
+* `derived-metadata` is an aggregate option that is equivalent to
+  `pack-metadata,commit-graph`.
+* `committed` is an aggregate option that is currently equivalent to
+  `objects`. This mode sacrifices some performance to ensure that work
+  that is committed to the repository with `git commit` or similar commands
+  is hardened.
+* `added` is an aggregate option that is currently equivalent to
+  `committed,index`. This mode sacrifices additional performance to
+  ensure that the results of commands like `git add` and similar operations
+  are hardened.
+* `all` is an aggregate option that syncs all individual components above.
+
 core.fsyncMethod::
 	A value indicating the strategy Git will use to harden repository data
 	using fsync and related primitives.
diff --git a/cache.h b/cache.h
index e08eeac6c1..86680f144e 100644
--- a/cache.h
+++ b/cache.h
@@ -1007,9 +1007,26 @@  enum fsync_component {
 	FSYNC_COMPONENT_INDEX			= 1 << 4,
 };
 
-#define FSYNC_COMPONENTS_DEFAULT (FSYNC_COMPONENT_PACK | \
-				  FSYNC_COMPONENT_PACK_METADATA | \
-				  FSYNC_COMPONENT_COMMIT_GRAPH)
+#define FSYNC_COMPONENTS_OBJECTS (FSYNC_COMPONENT_LOOSE_OBJECT | \
+				  FSYNC_COMPONENT_PACK)
+
+#define FSYNC_COMPONENTS_DERIVED_METADATA (FSYNC_COMPONENT_PACK_METADATA | \
+					   FSYNC_COMPONENT_COMMIT_GRAPH)
+
+#define FSYNC_COMPONENTS_DEFAULT (FSYNC_COMPONENTS_OBJECTS | \
+				  FSYNC_COMPONENTS_DERIVED_METADATA | \
+				  ~FSYNC_COMPONENT_LOOSE_OBJECT)
+
+#define FSYNC_COMPONENTS_COMMITTED (FSYNC_COMPONENTS_OBJECTS)
+
+#define FSYNC_COMPONENTS_ADDED (FSYNC_COMPONENTS_COMMITTED | \
+				FSYNC_COMPONENT_INDEX)
+
+#define FSYNC_COMPONENTS_ALL (FSYNC_COMPONENT_LOOSE_OBJECT | \
+			      FSYNC_COMPONENT_PACK | \
+			      FSYNC_COMPONENT_PACK_METADATA | \
+			      FSYNC_COMPONENT_COMMIT_GRAPH | \
+			      FSYNC_COMPONENT_INDEX)
 
 /*
  * A bitmask indicating which components of the repo should be fsynced.
diff --git a/config.c b/config.c
index 80f33c9198..fd8e165931 100644
--- a/config.c
+++ b/config.c
@@ -1332,6 +1332,11 @@  static const struct fsync_component_name {
 	{ "pack-metadata", FSYNC_COMPONENT_PACK_METADATA },
 	{ "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH },
 	{ "index", FSYNC_COMPONENT_INDEX },
+	{ "objects", FSYNC_COMPONENTS_OBJECTS },
+	{ "derived-metadata", FSYNC_COMPONENTS_DERIVED_METADATA },
+	{ "committed", FSYNC_COMPONENTS_COMMITTED },
+	{ "added", FSYNC_COMPONENTS_ADDED },
+	{ "all", FSYNC_COMPONENTS_ALL },
 };
 
 static enum fsync_component parse_fsync_components(const char *var, const char *string)