{"thread":{"id":"44768","subject":"[PATCH v3 00/21] Add configuration options for split-index","startedAt":"2016-12-26T10:22:41Z","lastAt":"2017-01-31T21:39:37Z","messageCount":56,"participants":["Christian Couder","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":3,"patchTotal":21},"messages":[{"id":"308336","messageId":"20161226102222.17150-1-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":null,"subject":"[PATCH v3 00/21] Add configuration options for split-index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:01Z","receivedAt":"2016-12-26T10:22:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Goal\n~~~~\n\nWe want to make it possible to use the split-index feature\nautomatically by just setting a new \"core.splitIndex\" configuration\nvariable to true.\n\nThis can be valuable as split-index can help significantly speed up\n`git rebase` especially along with the work to libify `git apply`\nthat has been merged to master\n(see https://github.com/git/git/commit/81358dc238372793b1590efa149cc1581d1fbd98)\nand is now in v2.11.\n\nDesign\n~~~~~~\n\nThe design is similar as the previous work that introduced\n\"core.untrackedCache\". \n\nThe new \"core.splitIndex\" configuration option can be either true,\nfalse or undefined which is the default.\n\nWhen it is true, the split index is created, if it does not already\nexists, when the index is read. When it is false, the split index is\nremoved if it exists, when the index is read. Otherwise it is left as\nis.\n\nAlong with this new configuration variable, the two following options\nare also introduced:\n\n - splitIndex.maxPercentChange\n\n This is to avoid having too many changes accumulating in the split\n index while in split index mode. The git-update-index\n documentation says:\n\n\tIf split-index mode is already enabled and `--split-index` is\n\tgiven again, all changes in $GIT_DIR/index are pushed back to\n\tthe shared index file.\n\n but it is probably better to not expect the user to think about it\n and to have a mechanism that pushes back all changes to the shared\n index file automatically when some threshold is reached.\n\n The default threshold is when the number of entries in the split\n index file reaches 20% of the number of entries in the shared\n index file. The new \"splitIndex.maxPercentChange\" config option\n lets people tweak this value.\n\n - splitIndex.sharedIndexExpire\n\n To make sure that old sharedindex files are eventually removed\n when a new one has been created, we \"touch\" the shared index file\n every time a split index file using the shared index file is\n either created or read from. Then we can delete shared indexes\n with an mtime older than one week (by default), when we create a\n new shared index file. The new \"splitIndex.sharedIndexExpire\"\n config option lets people tweak this grace period.\n\n This idea was suggested by Duy in:\n\n https://public-inbox.org/git/CACsJy8BqMFASHf5kJgUh+bd7XG98CafNydE964VJyPXz-emEvA@mail.gmail.com/\n\n and after some experiments, I agree that it is much simpler than\n what I thought could be done during our discussion.\n\n Junio also thinks that we have to do \"time-based GC\" in:\n \n https://public-inbox.org/git/xmqqeg33ccjj.fsf@gitster.mtv.corp.google.com/\n\nNote that this patch series doesn't address a leak when removing a\nsplit-index, but Duy wrote that he has a patch to fix this leak:\n\nhttps://public-inbox.org/git/CACsJy8AisF2ZVs7JdnVFp5wdskkbVQQQ=DBq5UzE1MOsCfBMtQ@mail.gmail.com/\n\nHighlevel view of the patches in the series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nExcept for patch 1/21, there are 3 big steps, one for each new\nconfiguration variable introduced.\n\nThere only a few small differences between this patch series and the\nv2 patch series sent a few weeks ago. Only two commits have been\nchanged a little, as suggested by Duy.\n\n - Patch 1/21 marks a message for translation. It was a new patch\n in v2 and it can be applied separately. (The patch 1/19 in v1,\n which was a typo fix, has been merged separately into master\n already.)\n\nStep 1 is:\n\n - Patches 2/21 to 5/21 introduce the functions that are reading\n the \"core.splitIndex\" configuration variable and tweaking the\n split index depending on its value.\n\n - Patch 6/21 adds a few tests for the new feature.\n\n - Patches 7/21 and 8/21 add some documentation for the new\n feature.\n\n The only change since v2 in this step is that an unnecessary\n variable initialization has been droped in 2/21, as suggested by\n Duy.\n\nStep 2 is:\n\n - Patches 9/21 and 10/21 introduce the functions that are reading\n the \"splitIndex.maxPercentChange\" configuration variable and\n regenerating a new shared index file depending on its value.\n\n - Patch 11/21 adds a few tests for the new feature.\n\n - Patch 12/21 add some documentation for the new feature.\n\n There are no changes in this step since v2.\n\nStep 3 is:\n\n - Patches 13/21 to 16/21 introduce the functions that are reading\n the \"splitIndex.sharedIndexExpire\" configuration variable and\n expiring old shared index files depending on its value.\n\n - Patch 17/21 adds a few tests for the new feature.\n\n - Patches 18/21 and 19/21 are new patches. They update the mtime\n of the shared index file when a split index based on the shared\n index file is read from. 18/21 is a refactoring to make the\n actual change in 19/21 easier.\n\n - Patches 20/21 and 21/21 add some documentation for the new\n feature.\n\n The only change since v2 in this step is that the full shared\n index path is passed to the can_delete_shared_index() function in\n 16/21 as suggested by Duy.\n\nLinks\n~~~~~\n\nThis patch series is also available here:\n\n https://github.com/chriscool/git/commits/config-split-index\n\nThe previous versions were:\n\n RFC: https://github.com/chriscool/git/commits/config-split-index7\n v1: https://github.com/chriscool/git/commits/config-split-index72\n v2: https://github.com/chriscool/git/commits/config-split-index99\n\nOn the mailing list the related patch series and discussions were:\n\n RFC: https://public-inbox.org/git/20160711172254.13439-1-chriscool@tuxfamily.org/\n v1: https://public-inbox.org/git/20161023092648.12086-1-chriscool@tuxfamily.org/\n v2: https://public-inbox.org/git/20161217145547.11748-1-chriscool@tuxfamily.org/\n\nChristian Couder (21):\n config: mark an error message up for translation\n config: add git_config_get_split_index()\n split-index: add {add,remove}_split_index() functions\n read-cache: add and then use tweak_split_index()\n update-index: warn in case of split-index incoherency\n t1700: add tests for core.splitIndex\n Documentation/config: add information for core.splitIndex\n Documentation/git-update-index: talk about core.splitIndex config var\n config: add git_config_get_max_percent_split_change()\n read-cache: regenerate shared index if necessary\n t1700: add tests for splitIndex.maxPercentChange\n Documentation/config: add splitIndex.maxPercentChange\n sha1_file: make check_and_freshen_file() non static\n read-cache: touch shared index files when used\n config: add git_config_get_expiry() from gc.c\n read-cache: unlink old sharedindex files\n t1700: test shared index file expiration\n read-cache: refactor read_index_from()\n read-cache: use freshen_shared_index() in read_index_from()\n Documentation/config: add splitIndex.sharedIndexExpire\n Documentation/git-update-index: explain splitIndex.*\n\n Documentation/config.txt | 28 +++++++\n Documentation/git-update-index.txt | 43 +++++++++--\n builtin/gc.c | 15 +---\n builtin/update-index.c | 25 +++---\n cache.h | 8 ++\n config.c | 42 +++++++++-\n read-cache.c | 143 ++++++++++++++++++++++++++++++++--\n sha1_file.c | 2 +-\n split-index.c | 22 ++++++\n split-index.h | 2 +\n t/t1700-split-index.sh | 154 +++++++++++++++++++++++++++++++++++++\n 11 files changed, 442 insertions(+), 42 deletions(-)\n\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308337","messageId":"20161226102222.17150-2-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 01/21] config: mark an error message up for translation","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:02Z","receivedAt":"2016-12-26T10:22:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n config.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 83fdecb1bc..2eaf8ad77a 100644\n--- a/config.c\n+++ b/config.c\n@@ -1701,8 +1701,8 @@ int git_config_get_untracked_cache(void)\n \t\tif (!strcasecmp(v, \"keep\"))\n \t\t\treturn -1;\n \n-\t\terror(\"unknown core.untrackedCache value '%s'; \"\n-\t\t \"using 'keep' default value\", v);\n+\t\terror(_(\"unknown core.untrackedCache value '%s'; \"\n+\t\t\t\"using 'keep' default value\"), v);\n \t\treturn -1;\n \t}\n \n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308338","messageId":"20161226102222.17150-3-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 02/21] config: add git_config_get_split_index()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:03Z","receivedAt":"2016-12-26T10:22:47Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new function will be used in a following commit to know\nif we want to use the split index feature or not.\n\nSigned-off-by: Christian Couder \n---\n cache.h | 1 +\n config.c | 10 ++++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex a50a61a197..c126fe475e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1821,6 +1821,7 @@ extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);\n extern int git_config_get_maybe_bool(const char *key, int *dest);\n extern int git_config_get_pathname(const char *key, const char **dest);\n extern int git_config_get_untracked_cache(void);\n+extern int git_config_get_split_index(void);\n \n /*\n * This is a hack for test programs like test-dump-untracked-cache to\ndiff --git a/config.c b/config.c\nindex 2eaf8ad77a..421e8c9da6 100644\n--- a/config.c\n+++ b/config.c\n@@ -1709,6 +1709,16 @@ int git_config_get_untracked_cache(void)\n \treturn -1; /* default value */\n }\n \n+int git_config_get_split_index(void)\n+{\n+\tint val;\n+\n+\tif (!git_config_get_maybe_bool(\"core.splitindex\", &val))\n+\t\treturn val;\n+\n+\treturn -1; /* default value */\n+}\n+\n NORETURN\n void git_die_config_linenr(const char *key, const char *filename, int linenr)\n {\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308339","messageId":"20161226102222.17150-4-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 03/21] split-index: add {add,remove}_split_index() functions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:04Z","receivedAt":"2016-12-26T10:22:50Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Also use the functions in cmd_update_index() in\nbuiltin/update-index.c.\n\nThese functions will be used in a following commit to tweak\nour use of the split-index feature depending on the setting\nof a configuration variable.\n\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 18 ++++++------------\n split-index.c | 22 ++++++++++++++++++++++\n split-index.h | 2 ++\n 3 files changed, 30 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex d530e89368..24fdadfa4b 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1099,18 +1099,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (split_index > 0) {\n-\t\tinit_split_index(&the_index);\n-\t\tthe_index.cache_changed |= SPLIT_INDEX_ORDERED;\n-\t} else if (!split_index && the_index.split_index) {\n-\t\t/*\n-\t\t * can't discard_split_index(&the_index); because that\n-\t\t * will destroy split_index->base->cache[], which may\n-\t\t * be shared with the_index.cache[]. So yeah we're\n-\t\t * leaking a bit here.\n-\t\t */\n-\t\tthe_index.split_index = NULL;\n-\t\tthe_index.cache_changed |= SOMETHING_CHANGED;\n-\t}\n+\t\tif (the_index.split_index)\n+\t\t\tthe_index.cache_changed |= SPLIT_INDEX_ORDERED;\n+\t\telse\n+\t\t\tadd_split_index(&the_index);\n+\t} else if (!split_index)\n+\t\tremove_split_index(&the_index);\n \n \tswitch (untracked_cache) {\n \tcase UC_UNSPECIFIED:\ndiff --git a/split-index.c b/split-index.c\nindex 615f4cac05..f519e60f87 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -317,3 +317,25 @@ void replace_index_entry_in_base(struct index_state *istate,\n \t\tistate->split_index->base->cache[new->index - 1] = new;\n \t}\n }\n+\n+void add_split_index(struct index_state *istate)\n+{\n+\tif (!istate->split_index) {\n+\t\tinit_split_index(istate);\n+\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+\t}\n+}\n+\n+void remove_split_index(struct index_state *istate)\n+{\n+\tif (istate->split_index) {\n+\t\t/*\n+\t\t * can't discard_split_index(&the_index); because that\n+\t\t * will destroy split_index->base->cache[], which may\n+\t\t * be shared with the_index.cache[]. So yeah we're\n+\t\t * leaking a bit here.\n+\t\t */\n+\t\tistate->split_index = NULL;\n+\t\tistate->cache_changed |= SOMETHING_CHANGED;\n+\t}\n+}\ndiff --git a/split-index.h b/split-index.h\nindex c1324f521a..df91c1bda8 100644\n--- a/split-index.h\n+++ b/split-index.h\n@@ -31,5 +31,7 @@ void merge_base_index(struct index_state *istate);\n void prepare_to_write_split_index(struct index_state *istate);\n void finish_writing_split_index(struct index_state *istate);\n void discard_split_index(struct index_state *istate);\n+void add_split_index(struct index_state *istate);\n+void remove_split_index(struct index_state *istate);\n \n #endif\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308340","messageId":"20161226102222.17150-5-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 04/21] read-cache: add and then use tweak_split_index()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:05Z","receivedAt":"2016-12-26T10:22:55Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This will make us use the split-index feature or not depending\non the value of the \"core.splitIndex\" config variable.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex f92a912dcb..732b7fe611 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1566,10 +1566,27 @@ static void tweak_untracked_cache(struct index_state *istate)\n \t}\n }\n \n+static void tweak_split_index(struct index_state *istate)\n+{\n+\tswitch (git_config_get_split_index()) {\n+\tcase -1: /* unset: do nothing */\n+\t\tbreak;\n+\tcase 0: /* false */\n+\t\tremove_split_index(istate);\n+\t\tbreak;\n+\tcase 1: /* true */\n+\t\tadd_split_index(istate);\n+\t\tbreak;\n+\tdefault: /* unknown value: do nothing */\n+\t\tbreak;\n+\t}\n+}\n+\n static void post_read_index_from(struct index_state *istate)\n {\n \tcheck_ce_order(istate);\n \ttweak_untracked_cache(istate);\n+\ttweak_split_index(istate);\n }\n \n /* remember to discard_cache() before reading a different cache! */\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308341","messageId":"20161226102222.17150-10-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 09/21] config: add git_config_get_max_percent_split_change()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:10Z","receivedAt":"2016-12-26T10:22:58Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new function will be used in a following commit to get the\nvalue of the \"splitIndex.maxPercentChange\" config variable.\n\nSigned-off-by: Christian Couder \n---\n cache.h | 1 +\n config.c | 15 +++++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex c126fe475e..e15b421b6f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1822,6 +1822,7 @@ extern int git_config_get_maybe_bool(const char *key, int *dest);\n extern int git_config_get_pathname(const char *key, const char **dest);\n extern int git_config_get_untracked_cache(void);\n extern int git_config_get_split_index(void);\n+extern int git_config_get_max_percent_split_change(void);\n \n /*\n * This is a hack for test programs like test-dump-untracked-cache to\ndiff --git a/config.c b/config.c\nindex 421e8c9da6..cf212785bb 100644\n--- a/config.c\n+++ b/config.c\n@@ -1719,6 +1719,21 @@ int git_config_get_split_index(void)\n \treturn -1; /* default value */\n }\n \n+int git_config_get_max_percent_split_change(void)\n+{\n+\tint val = -1;\n+\n+\tif (!git_config_get_int(\"splitindex.maxpercentchange\", &val)) {\n+\t\tif (0 <= val && val <= 100)\n+\t\t\treturn val;\n+\n+\t\treturn error(_(\"splitIndex.maxPercentChange value '%d' \"\n+\t\t\t \"should be between 0 and 100\"), val);\n+\t}\n+\n+\treturn -1; /* default value */\n+}\n+\n NORETURN\n void git_die_config_linenr(const char *key, const char *filename, int linenr)\n {\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308342","messageId":"20161226102222.17150-11-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 10/21] read-cache: regenerate shared index if necessary","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:11Z","receivedAt":"2016-12-26T10:23:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When writing a new split-index and there is a big number of cache\nentries in the split-index compared to the shared index, it is a\ngood idea to regenerate the shared index.\n\nBy default when the ratio reaches 20%, we will push back all\nthe entries from the split-index into a new shared index file\ninstead of just creating a new split-index file.\n\nThe threshold can be configured using the\n\"splitIndex.maxPercentChange\" config variable.\n\nWe need to adjust the existing tests in t1700 by setting\n\"splitIndex.maxPercentChange\" to 100 at the beginning of t1700,\nas the existing tests are assuming that the shared index is\nregenerated only when `git update-index --split-index` is used.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 32 ++++++++++++++++++++++++++++++++\n t/t1700-split-index.sh | 1 +\n 2 files changed, 33 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 732b7fe611..a1aaec5135 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2220,6 +2220,36 @@ static int write_shared_index(struct index_state *istate,\n \treturn ret;\n }\n \n+static const int default_max_percent_split_change = 20;\n+\n+static int too_many_not_shared_entries(struct index_state *istate)\n+{\n+\tint i, not_shared = 0;\n+\tint max_split = git_config_get_max_percent_split_change();\n+\n+\tswitch (max_split) {\n+\tcase -1:\n+\t\t/* not or badly configured: use the default value */\n+\t\tmax_split = default_max_percent_split_change;\n+\t\tbreak;\n+\tcase 0:\n+\t\treturn 1; /* 0% means always write a new shared index */\n+\tcase 100:\n+\t\treturn 0; /* 100% means never write a new shared index */\n+\tdefault:\n+\t\t; /* do nothing: just use the configured value */\n+\t}\n+\n+\t/* Count not shared entries */\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tif (!ce->index)\n+\t\t\tnot_shared++;\n+\t}\n+\n+\treturn istate->cache_nr * max_split < not_shared * 100;\n+}\n+\n int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\t unsigned flags)\n {\n@@ -2237,6 +2267,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tif ((v & 15) < 6)\n \t\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n \t}\n+\tif (too_many_not_shared_entries(istate))\n+\t\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n \tif (istate->cache_changed & SPLIT_INDEX_ORDERED) {\n \t\tint ret = write_shared_index(istate, lock, flags);\n \t\tif (ret)\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex db8c39f446..507a1dd1ad 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -8,6 +8,7 @@ test_description='split index mode tests'\n sane_unset GIT_TEST_SPLIT_INDEX\n \n test_expect_success 'enable split index' '\n+\tgit config splitIndex.maxPercentChange 100 &&\n \tgit update-index --split-index &&\n \ttest-dump-split-index .git/index >actual &&\n \tindexversion=$(test-index-version <.git/index) &&\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308343","messageId":"20161226102222.17150-6-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 05/21] update-index: warn in case of split-index incoherency","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:06Z","receivedAt":"2016-12-26T10:23:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When users are using `git update-index --(no-)split-index`, they\nmay expect the split-index feature to be used or not according to\nthe option they just used, but this might not be the case if the\nnew \"core.splitIndex\" config variable has been set. In this case\nlet's warn about what will happen and why.\n\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 24fdadfa4b..d74d72cc7f 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1099,12 +1099,21 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (split_index > 0) {\n+\t\tif (git_config_get_split_index() == 0)\n+\t\t\twarning(_(\"core.splitIndex is set to false; \"\n+\t\t\t\t \"remove or change it, if you really want to \"\n+\t\t\t\t \"enable split index\"));\n \t\tif (the_index.split_index)\n \t\t\tthe_index.cache_changed |= SPLIT_INDEX_ORDERED;\n \t\telse\n \t\t\tadd_split_index(&the_index);\n-\t} else if (!split_index)\n+\t} else if (!split_index) {\n+\t\tif (git_config_get_split_index() == 1)\n+\t\t\twarning(_(\"core.splitIndex is set to true; \"\n+\t\t\t\t \"remove or change it, if you really want to \"\n+\t\t\t\t \"disable split index\"));\n \t\tremove_split_index(&the_index);\n+\t}\n \n \tswitch (untracked_cache) {\n \tcase UC_UNSPECIFIED:\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308344","messageId":"20161226102222.17150-12-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 11/21] t1700: add tests for splitIndex.maxPercentChange","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:12Z","receivedAt":"2016-12-26T10:23:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t1700-split-index.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 72 insertions(+)\n\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 507a1dd1ad..f03addf654 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -238,4 +238,76 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'set core.splitIndex config variable to true' '\n+\tgit config core.splitIndex true &&\n+\t: >three &&\n+\tgit update-index --add three &&\n+\tBASE=$(test-dump-split-index .git/index | grep \"^base\") &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <four &&\n+\tgit update-index --add four &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <five &&\n+\tgit update-index --add five &&\n+\tBASE=$(test-dump-split-index .git/index | grep \"^base\") &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <six &&\n+\tgit update-index --add six &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <seven &&\n+\tgit update-index --add seven &&\n+\tBASE=$(test-dump-split-index .git/index | grep \"^base\") &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <eight &&\n+\tgit update-index --add eight &&\n+\tBASE=$(test-dump-split-index .git/index | grep \"^base\") &&\n+\ttest-dump-split-index .git/index | sed \"/^own/d\" >actual &&\n+\tcat >expect <\n---\n builtin/gc.c | 15 ++-------------\n cache.h | 3 +++\n config.c | 13 +++++++++++++\n 3 files changed, 18 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 069950d0b4..1e40d45aa2 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -62,17 +62,6 @@ static void report_pack_garbage(unsigned seen_bits, const char *path)\n \t\tstring_list_append(&pack_garbage, path);\n }\n \n-static void git_config_date_string(const char *key, const char **output)\n-{\n-\tif (git_config_get_string_const(key, output))\n-\t\treturn;\n-\tif (strcmp(*output, \"now\")) {\n-\t\tunsigned long now = approxidate(\"now\");\n-\t\tif (approxidate(*output) >= now)\n-\t\t\tgit_die_config(key, _(\"Invalid %s: '%s'\"), key, *output);\n-\t}\n-}\n-\n static void process_log_file(void)\n {\n \tstruct stat st;\n@@ -111,8 +100,8 @@ static void gc_config(void)\n \tgit_config_get_int(\"gc.auto\", &gc_auto_threshold);\n \tgit_config_get_int(\"gc.autopacklimit\", &gc_auto_pack_limit);\n \tgit_config_get_bool(\"gc.autodetach\", &detach_auto);\n-\tgit_config_date_string(\"gc.pruneexpire\", &prune_expire);\n-\tgit_config_date_string(\"gc.worktreepruneexpire\", &prune_worktrees_expire);\n+\tgit_config_get_expiry(\"gc.pruneexpire\", &prune_expire);\n+\tgit_config_get_expiry(\"gc.worktreepruneexpire\", &prune_worktrees_expire);\n \tgit_config(git_default_config, NULL);\n }\n \ndiff --git a/cache.h b/cache.h\nindex f442f28189..279415afbd 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1827,6 +1827,9 @@ extern int git_config_get_untracked_cache(void);\n extern int git_config_get_split_index(void);\n extern int git_config_get_max_percent_split_change(void);\n \n+/* This dies if the configured or default date is in the future */\n+extern int git_config_get_expiry(const char *key, const char **output);\n+\n /*\n * This is a hack for test programs like test-dump-untracked-cache to\n * ensure that they do not modify the untracked cache when reading it.\ndiff --git a/config.c b/config.c\nindex cf212785bb..d6c8f8f3ba 100644\n--- a/config.c\n+++ b/config.c\n@@ -1685,6 +1685,19 @@ int git_config_get_pathname(const char *key, const char **dest)\n \treturn ret;\n }\n \n+int git_config_get_expiry(const char *key, const char **output)\n+{\n+\tint ret = git_config_get_string_const(key, output);\n+\tif (ret)\n+\t\treturn ret;\n+\tif (strcmp(*output, \"now\")) {\n+\t\tunsigned long now = approxidate(\"now\");\n+\t\tif (approxidate(*output) >= now)\n+\t\t\tgit_die_config(key, _(\"Invalid %s: '%s'\"), key, *output);\n+\t}\n+\treturn ret;\n+}\n+\n int git_config_get_untracked_cache(void)\n {\n \tint val = -1;\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308346","messageId":"20161226102222.17150-13-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:13Z","receivedAt":"2016-12-26T10:23:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/config.txt | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 221c5982c0..e0f5a77980 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2773,6 +2773,19 @@ showbranch.default::\n \tThe default set of branches for linkgit:git-show-branch[1].\n \tSee linkgit:git-show-branch[1].\n \n+splitIndex.maxPercentChange::\n+\tWhen the split index feature is used, this specifies the\n+\tpercent of entries the split index can contain compared to the\n+\twhole number of entries in both the split index and the shared\n+\tindex before a new shared index is written.\n+\tThe value should be between 0 and 100. If the value is 0 then\n+\ta new shared index is always written, if it is 100 a new\n+\tshared index is never written.\n+\tBy default the value is 20, so a new shared index is written\n+\tif the number of entries in the split index would be greater\n+\tthan 20 percent of the total number of entries.\n+\tSee linkgit:git-update-index[1].\n+\n status.relativePaths::\n \tBy default, linkgit:git-status[1] shows paths relative to the\n \tcurrent directory. Setting this variable to `false` shows paths\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308347","messageId":"20161226102222.17150-20-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 19/21] read-cache: use freshen_shared_index() in read_index_from()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:20Z","receivedAt":"2016-12-26T10:23:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This way a share index file will not be garbage collected if\nwe still read from an index it is based from.\n\nAs we need to read the current index before creating a new\none, the tests have to be adjusted, so that we don't expect\nan old shared index file to be deleted right away when we\ncreate a new one.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 1 +\n t/t1700-split-index.sh | 14 +++++++-------\n 2 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 98ef1323d6..bf0ac1ce61 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1727,6 +1727,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\t base_sha1_hex, base_path,\n \t\t sha1_to_hex(split_index->base->sha1));\n \n+\tfreshen_shared_index(base_sha1_hex);\n \tmerge_base_index(istate);\n \tpost_read_index_from(istate);\n \treturn ret;\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex f448fc13cd..800f84a593 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -313,17 +313,17 @@ EOF\n test_expect_success 'shared index files expire after 7 days by default' '\n \t: >ten &&\n \tgit update-index --add ten &&\n-\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 2 &&\n \tjust_under_7_days_ago=$((1-7*86400)) &&\n \ttest-chmtime =$just_under_7_days_ago .git/sharedindex.* &&\n \t: >eleven &&\n \tgit update-index --add eleven &&\n-\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 2 &&\n \tjust_over_7_days_ago=$((-1-7*86400)) &&\n \ttest-chmtime =$just_over_7_days_ago .git/sharedindex.* &&\n \t: >twelve &&\n \tgit update-index --add twelve &&\n-\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+\ttest $(ls .git/sharedindex.* | wc -l) -le 2\n '\n \n test_expect_success 'check splitIndex.sharedIndexExpire set to 8 days' '\n@@ -331,12 +331,12 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to 8 days' '\n \ttest-chmtime =$just_over_7_days_ago .git/sharedindex.* &&\n \t: >thirteen &&\n \tgit update-index --add thirteen &&\n-\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 2 &&\n \tjust_over_8_days_ago=$((-1-8*86400)) &&\n \ttest-chmtime =$just_over_8_days_ago .git/sharedindex.* &&\n \t: >fourteen &&\n \tgit update-index --add fourteen &&\n-\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+\ttest $(ls .git/sharedindex.* | wc -l) -le 2\n '\n \n test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"' '\n@@ -345,13 +345,13 @@ test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"\n \ttest-chmtime =$just_10_years_ago .git/sharedindex.* &&\n \t: >fifteen &&\n \tgit update-index --add fifteen &&\n-\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 2 &&\n \tgit config splitIndex.sharedIndexExpire now &&\n \tjust_1_second_ago=-1 &&\n \ttest-chmtime =$just_1_second_ago .git/sharedindex.* &&\n \t: >sixteen &&\n \tgit update-index --add sixteen &&\n-\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+\ttest $(ls .git/sharedindex.* | wc -l) -le 2\n '\n \n test_done\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308348","messageId":"20161226102222.17150-21-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:21Z","receivedAt":"2016-12-26T10:23:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/config.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e0f5a77980..24e771d22e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2786,6 +2786,17 @@ splitIndex.maxPercentChange::\n \tthan 20 percent of the total number of entries.\n \tSee linkgit:git-update-index[1].\n \n+splitIndex.sharedIndexExpire::\n+\tWhen the split index feature is used, shared index files with\n+\ta mtime older than this time will be removed when a new shared\n+\tindex file is created. The value \"now\" expires all entries\n+\timmediately, and \"never\" suppresses expiration altogether.\n+\tThe default value is \"one.week.ago\".\n+\tNote that each time a new split-index file is created, the\n+\tmtime of the related shared index file is updated to the\n+\tcurrent time.\n+\tSee linkgit:git-update-index[1].\n+\n status.relativePaths::\n \tBy default, linkgit:git-status[1] shows paths relative to the\n \tcurrent directory. Setting this variable to `false` shows paths\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308349","messageId":"20161226102222.17150-18-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 17/21] t1700: test shared index file expiration","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:18Z","receivedAt":"2016-12-26T10:23:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t1700-split-index.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 44 insertions(+)\n\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex f03addf654..f448fc13cd 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -310,4 +310,48 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'shared index files expire after 7 days by default' '\n+\t: >ten &&\n+\tgit update-index --add ten &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\tjust_under_7_days_ago=$((1-7*86400)) &&\n+\ttest-chmtime =$just_under_7_days_ago .git/sharedindex.* &&\n+\t: >eleven &&\n+\tgit update-index --add eleven &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\tjust_over_7_days_ago=$((-1-7*86400)) &&\n+\ttest-chmtime =$just_over_7_days_ago .git/sharedindex.* &&\n+\t: >twelve &&\n+\tgit update-index --add twelve &&\n+\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+'\n+\n+test_expect_success 'check splitIndex.sharedIndexExpire set to 8 days' '\n+\tgit config splitIndex.sharedIndexExpire \"8.days.ago\" &&\n+\ttest-chmtime =$just_over_7_days_ago .git/sharedindex.* &&\n+\t: >thirteen &&\n+\tgit update-index --add thirteen &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\tjust_over_8_days_ago=$((-1-8*86400)) &&\n+\ttest-chmtime =$just_over_8_days_ago .git/sharedindex.* &&\n+\t: >fourteen &&\n+\tgit update-index --add fourteen &&\n+\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+'\n+\n+test_expect_success 'check splitIndex.sharedIndexExpire set to \"never\" and \"now\"' '\n+\tgit config splitIndex.sharedIndexExpire never &&\n+\tjust_10_years_ago=$((-365*10*86400)) &&\n+\ttest-chmtime =$just_10_years_ago .git/sharedindex.* &&\n+\t: >fifteen &&\n+\tgit update-index --add fifteen &&\n+\ttest $(ls .git/sharedindex.* | wc -l) -gt 1 &&\n+\tgit config splitIndex.sharedIndexExpire now &&\n+\tjust_1_second_ago=-1 &&\n+\ttest-chmtime =$just_1_second_ago .git/sharedindex.* &&\n+\t: >sixteen &&\n+\tgit update-index --add sixteen &&\n+\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+'\n+\n test_done\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308350","messageId":"20161226102222.17150-22-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 21/21] Documentation/git-update-index: explain splitIndex.*","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:22Z","receivedAt":"2016-12-26T10:23:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/config.txt | 6 +++---\n Documentation/git-update-index.txt | 37 +++++++++++++++++++++++++++++--------\n 2 files changed, 32 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 24e771d22e..87a570cf88 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2792,9 +2792,9 @@ splitIndex.sharedIndexExpire::\n \tindex file is created. The value \"now\" expires all entries\n \timmediately, and \"never\" suppresses expiration altogether.\n \tThe default value is \"one.week.ago\".\n-\tNote that each time a new split-index file is created, the\n-\tmtime of the related shared index file is updated to the\n-\tcurrent time.\n+\tNote that each time a split index based on a shared index file\n+\tis either created or read from, the mtime of the shared index\n+\tfile is updated to the current time.\n \tSee linkgit:git-update-index[1].\n \n status.relativePaths::\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex e091b2a409..46c953b2f2 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -163,14 +163,10 @@ may not support it yet.\n \n --split-index::\n --no-split-index::\n-\tEnable or disable split index mode. If enabled, the index is\n-\tsplit into two files, $GIT_DIR/index and $GIT_DIR/sharedindex..\n-\tChanges are accumulated in $GIT_DIR/index while the shared\n-\tindex file contains all index entries stays unchanged. If\n-\tsplit-index mode is already enabled and `--split-index` is\n-\tgiven again, all changes in $GIT_DIR/index are pushed back to\n-\tthe shared index file. This mode is designed for very large\n-\tindexes that take a significant amount of time to read or write.\n+\tEnable or disable split index mode. If split-index mode is\n+\talready enabled and `--split-index` is given again, all\n+\tchanges in $GIT_DIR/index are pushed back to the shared index\n+\tfile.\n +\n These options take effect whatever the value of the `core.splitIndex`\n configuration variable (see linkgit:git-config[1]). But a warning is\n@@ -394,6 +390,31 @@ Although this bit looks similar to assume-unchanged bit, its goal is\n different from assume-unchanged bit's. Skip-worktree also takes\n precedence over assume-unchanged bit when both are set.\n \n+Split index\n+-----------\n+\n+This mode is designed for very large indexes that take a significant\n+amount of time to read or write.\n+\n+In this mode, the index is split into two files, $GIT_DIR/index and\n+$GIT_DIR/sharedindex.. Changes are accumulated in\n+$GIT_DIR/index, the split index, while the shared index file contains\n+all index entries and stays unchanged.\n+\n+All changes in the split index are pushed back to the shared index\n+file when the number of entries in the split index reaches a level\n+specified by the splitIndex.maxPercentChange config variable (see\n+linkgit:git-config[1]).\n+\n+Each time a new shared index file is created, the old shared index\n+files are deleted if their mtime is older than what is specified by\n+the splitIndex.sharedIndexExpire config variable (see\n+linkgit:git-config[1]).\n+\n+To avoid deleting a shared index file that is still used, its mtime is\n+updated to the current time everytime a new split index based on the\n+shared index file is either created or read from.\n+\n Untracked cache\n ---------------\n \n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308351","messageId":"20161226102222.17150-19-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 18/21] read-cache: refactor read_index_from()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:19Z","receivedAt":"2016-12-26T10:23:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It looks better and is simpler to review when we don't compute\nthe same things many times in the function.\n\nIt will also help make the following commit simpler.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex e62a6c742d..98ef1323d6 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1699,6 +1699,8 @@ int read_index_from(struct index_state *istate, const char *path)\n {\n \tstruct split_index *split_index;\n \tint ret;\n+\tchar *base_sha1_hex;\n+\tconst char *base_path;\n \n \t/* istate->initialized covers both .git/index and .git/sharedindex.xxx */\n \tif (istate->initialized)\n@@ -1716,15 +1718,15 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\tdiscard_index(split_index->base);\n \telse\n \t\tsplit_index->base = xcalloc(1, sizeof(*split_index->base));\n-\tret = do_read_index(split_index->base,\n-\t\t\t git_path(\"sharedindex.%s\",\n-\t\t\t\t sha1_to_hex(split_index->base_sha1)), 1);\n+\n+\tbase_sha1_hex = sha1_to_hex(split_index->base_sha1);\n+\tbase_path = git_path(\"sharedindex.%s\", base_sha1_hex);\n+\tret = do_read_index(split_index->base, base_path, 1);\n \tif (hashcmp(split_index->base_sha1, split_index->base->sha1))\n \t\tdie(\"broken index, expect %s in %s, got %s\",\n-\t\t sha1_to_hex(split_index->base_sha1),\n-\t\t git_path(\"sharedindex.%s\",\n-\t\t\t sha1_to_hex(split_index->base_sha1)),\n+\t\t base_sha1_hex, base_path,\n \t\t sha1_to_hex(split_index->base->sha1));\n+\n \tmerge_base_index(istate);\n \tpost_read_index_from(istate);\n \treturn ret;\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308352","messageId":"20161226102222.17150-17-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 16/21] read-cache: unlink old sharedindex files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:17Z","receivedAt":"2016-12-26T10:23:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Everytime split index is turned on, it creates a \"sharedindex.XXXX\"\nfile in the git directory. This change makes sure that shared index\nfiles that haven't been used for a long time are removed when a new\nshared index file is created.\n\nThe new \"splitIndex.sharedIndexExpire\" config variable is created\nto tell the delay after which an unused shared index file can be\ndeleted. It defaults to \"1.week.ago\".\n\nA previous commit made sure that each time a split index file is\ncreated the mtime of the shared index file it references is updated.\nThis makes sure that recently used shared index file will not be\ndeleted.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 63 insertions(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 9fbad2044b..e62a6c742d 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2207,6 +2207,65 @@ static int write_split_index(struct index_state *istate,\n \treturn ret;\n }\n \n+static const char *shared_index_expire = \"1.week.ago\";\n+\n+static unsigned long get_shared_index_expire_date(void)\n+{\n+\tstatic unsigned long shared_index_expire_date;\n+\tstatic int shared_index_expire_date_prepared;\n+\n+\tif (!shared_index_expire_date_prepared) {\n+\t\tgit_config_get_expiry(\"splitindex.sharedindexexpire\",\n+\t\t\t\t &shared_index_expire);\n+\t\tshared_index_expire_date = approxidate(shared_index_expire);\n+\t\tshared_index_expire_date_prepared = 1;\n+\t}\n+\n+\treturn shared_index_expire_date;\n+}\n+\n+static int can_delete_shared_index(const char *shared_index_path)\n+{\n+\tstruct stat st;\n+\tunsigned long expiration;\n+\n+\t/* Check timestamp */\n+\texpiration = get_shared_index_expire_date();\n+\tif (!expiration)\n+\t\treturn 0;\n+\tif (stat(shared_index_path, &st))\n+\t\treturn error_errno(_(\"could not stat '%s\"), shared_index_path);\n+\tif (st.st_mtime > expiration)\n+\t\treturn 0;\n+\n+\treturn 1;\n+}\n+\n+static int clean_shared_index_files(const char *current_hex)\n+{\n+\tstruct dirent *de;\n+\tDIR *dir = opendir(get_git_dir());\n+\n+\tif (!dir)\n+\t\treturn error_errno(_(\"unable to open git dir: %s\"), get_git_dir());\n+\n+\twhile ((de = readdir(dir)) != NULL) {\n+\t\tconst char *sha1_hex;\n+\t\tconst char *shared_index_path;\n+\t\tif (!skip_prefix(de->d_name, \"sharedindex.\", &sha1_hex))\n+\t\t\tcontinue;\n+\t\tif (!strcmp(sha1_hex, current_hex))\n+\t\t\tcontinue;\n+\t\tshared_index_path = git_path(\"%s\", de->d_name);\n+\t\tif (can_delete_shared_index(shared_index_path) > 0 &&\n+\t\t unlink(shared_index_path))\n+\t\t\terror_errno(_(\"unable to unlink: %s\"), shared_index_path);\n+\t}\n+\tclosedir(dir);\n+\n+\treturn 0;\n+}\n+\n static struct tempfile temporary_sharedindex;\n \n static int write_shared_index(struct index_state *istate,\n@@ -2228,8 +2287,11 @@ static int write_shared_index(struct index_state *istate,\n \t}\n \tret = rename_tempfile(&temporary_sharedindex,\n \t\t\t git_path(\"sharedindex.%s\", sha1_to_hex(si->base->sha1)));\n-\tif (!ret)\n+\tif (!ret) {\n \t\thashcpy(si->base_sha1, si->base->sha1);\n+\t\tclean_shared_index_files(sha1_to_hex(si->base->sha1));\n+\t}\n+\n \treturn ret;\n }\n \n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308353","messageId":"20161226102222.17150-15-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:15Z","receivedAt":"2016-12-26T10:23:26Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"When a split-index file is created, let's update the mtime of the\nshared index file that the split-index file is referencing.\n\nIn a following commit we will make shared index file expire\ndepending on their mtime, so updating the mtime makes sure that\nthe shared index file will not be deleted soon.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex a1aaec5135..9fbad2044b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1682,6 +1682,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tdie(\"index file corrupt\");\n }\n \n+/*\n+ * Signal that the shared index is used by updating its mtime.\n+ *\n+ * This way, shared index can be removed if they have not been used\n+ * for some time. It's ok to fail to update the mtime if we are on a\n+ * read only file system.\n+ */\n+void freshen_shared_index(char *base_sha1_hex)\n+{\n+\tconst char *shared_index = git_path(\"sharedindex.%s\", base_sha1_hex);\n+\tcheck_and_freshen_file(shared_index, 1);\n+}\n+\n int read_index_from(struct index_state *istate, const char *path)\n {\n \tstruct split_index *split_index;\n@@ -2273,6 +2286,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n \t\tint ret = write_shared_index(istate, lock, flags);\n \t\tif (ret)\n \t\t\treturn ret;\n+\t} else {\n+\t\tfreshen_shared_index(sha1_to_hex(si->base_sha1));\n \t}\n \n \treturn write_split_index(istate, lock, flags);\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308354","messageId":"20161226102222.17150-14-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 13/21] sha1_file: make check_and_freshen_file() non static","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:14Z","receivedAt":"2016-12-26T10:23:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This function will be used in a commit soon, so let's make\nit available globally.\n\nSigned-off-by: Christian Couder \n---\n cache.h | 3 +++\n sha1_file.c | 2 +-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex e15b421b6f..f442f28189 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1170,6 +1170,9 @@ extern int has_pack_index(const unsigned char *sha1);\n \n extern void assert_sha1_type(const unsigned char *sha1, enum object_type expect);\n \n+/* Helper to check and \"touch\" a file */\n+extern int check_and_freshen_file(const char *fn, int freshen);\n+\n extern const signed char hexval_table[256];\n static inline unsigned int hexval(unsigned char c)\n {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 1173071859..f5303d955a 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -601,7 +601,7 @@ static int freshen_file(const char *fn)\n * either does not exist on disk, or has a stale mtime and may be subject to\n * pruning).\n */\n-static int check_and_freshen_file(const char *fn, int freshen)\n+int check_and_freshen_file(const char *fn, int freshen)\n {\n \tif (access(fn, F_OK))\n \t\treturn 0;\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308355","messageId":"20161226102222.17150-8-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 07/21] Documentation/config: add information for core.splitIndex","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:08Z","receivedAt":"2016-12-26T10:23:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/config.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d51182a060..221c5982c0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -331,6 +331,10 @@ core.trustctime::\n \tcrawlers and some backup systems).\n \tSee linkgit:git-update-index[1]. True by default.\n \n+core.splitIndex::\n+\tIf true, the split-index feature of the index will be used.\n+\tSee linkgit:git-update-index[1]. False by default.\n+\n core.untrackedCache::\n \tDetermines what to do about the untracked cache feature of the\n \tindex. It will be kept, if this variable is unset or set to\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308356","messageId":"20161226102222.17150-9-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:09Z","receivedAt":"2016-12-26T10:23:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n Documentation/git-update-index.txt | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex 7386c93162..e091b2a409 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -171,6 +171,12 @@ may not support it yet.\n \tgiven again, all changes in $GIT_DIR/index are pushed back to\n \tthe shared index file. This mode is designed for very large\n \tindexes that take a significant amount of time to read or write.\n++\n+These options take effect whatever the value of the `core.splitIndex`\n+configuration variable (see linkgit:git-config[1]). But a warning is\n+emitted when the change goes against the configured value, as the\n+configured value will take effect next time the index is read and this\n+will remove the intended effect of the option.\n \n --untracked-cache::\n --no-untracked-cache::\n-- \n2.11.0.209.gda91e66374.dirty\n\n"},{"id":"308357","messageId":"20161226102222.17150-7-chriscool@tuxfamily.org","threadId":"44768","inReplyTo":"20161226102222.17150-1-chriscool@tuxfamily.org","subject":"[PATCH v3 06/21] t1700: add tests for core.splitIndex","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-12-26T10:22:07Z","receivedAt":"2016-12-26T10:23:38Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder \n---\n t/t1700-split-index.sh | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex 292a0720fc..db8c39f446 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -200,4 +200,41 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'set core.splitIndex config variable to true' '\n+\tgit config core.splitIndex true &&\n+\t: >three &&\n+\tgit update-index --add three &&\n+\tgit ls-files --stage >ls-files.actual &&\n+\tcat >ls-files.expect <actual &&\n+\tcat >expect <ls-files.actual &&\n+\tcat >ls-files.expect <actual &&\n+\tcat >expect < wrote:\n>\n> Highlevel view of the patches in the series\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n>\n> Except for patch 1/21, there are 3 big steps, one for each new\n> configuration variable introduced.\n>\n> There only a few small differences between this patch series and the\n> v2 patch series sent a few weeks ago. Only two commits have been\n> changed a little, as suggested by Duy.\n\nHere is the diff compared to v2:\n\ndiff --git a/config.c b/config.c\nindex 5c52cefd78..d6c8f8f3ba 100644\n--- a/config.c\n+++ b/config.c\n@@ -1724,7 +1724,7 @@ int git_config_get_untracked_cache(void)\n\n int git_config_get_split_index(void)\n {\n- int val = -1;\n+ int val;\n\n if (!git_config_get_maybe_bool(\"core.splitindex\", &val))\n return val;\ndiff --git a/read-cache.c b/read-cache.c\nindex 772343ab25..bf0ac1ce61 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2227,18 +2227,17 @@ static unsigned long get_shared_index_expire_date(void)\n return shared_index_expire_date;\n }\n\n-static int can_delete_shared_index(const char *shared_sha1_hex)\n+static int can_delete_shared_index(const char *shared_index_path)\n {\n struct stat st;\n unsigned long expiration;\n- const char *shared_index = git_path(\"sharedindex.%s\", shared_sha1_hex);\n\n /* Check timestamp */\n expiration = get_shared_index_expire_date();\n if (!expiration)\n return 0;\n- if (stat(shared_index, &st))\n- return error_errno(_(\"could not stat '%s\"), shared_index);\n+ if (stat(shared_index_path, &st))\n+ return error_errno(_(\"could not stat '%s\"), shared_index_path);\n if (st.st_mtime > expiration)\n return 0;\n\n@@ -2255,13 +2254,15 @@ static int clean_shared_index_files(const char\n*current_hex)\n\n while ((de = readdir(dir)) != NULL) {\n const char *sha1_hex;\n+ const char *shared_index_path;\n if (!skip_prefix(de->d_name, \"sharedindex.\", &sha1_hex))\n continue;\n if (!strcmp(sha1_hex, current_hex))\n continue;\n- if (can_delete_shared_index(sha1_hex) > 0 &&\n- unlink(git_path(\"%s\", de->d_name)))\n- error_errno(_(\"unable to unlink: %s\"),\ngit_path(\"%s\", de->d_name));\n+ shared_index_path = git_path(\"%s\", de->d_name);\n+ if (can_delete_shared_index(shared_index_path) > 0 &&\n+ unlink(shared_index_path))\n+ error_errno(_(\"unable to unlink: %s\"),\nshared_index_path);\n }\n closedir(dir);\n"},{"id":"308400","messageId":"xmqqlgv1b34y.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-13-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:09:17Z","receivedAt":"2016-12-27T19:09:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/config.txt | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 221c5982c0..e0f5a77980 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2773,6 +2773,19 @@ showbranch.default::\n> \tThe default set of branches for linkgit:git-show-branch[1].\n> \tSee linkgit:git-show-branch[1].\n> \n> +splitIndex.maxPercentChange::\n> +\tWhen the split index feature is used, this specifies the\n> +\tpercent of entries the split index can contain compared to the\n> +\twhole number of entries in both the split index and the shared\n\ns/whole/total/ to match the last sentence of this section, perhaps?\n\"The number of all entries\" would also be OK, but \"the whole number\nof entries\" sounds a bit strange.\n"},{"id":"308401","messageId":"xmqqh95pb348.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-14-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 13/21] sha1_file: make check_and_freshen_file() non static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:09:43Z","receivedAt":"2016-12-27T19:09:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> This function will be used in a commit soon, so let's make\n> it available globally.\n\nSee comment on 14/21; I am not convinced that this is the function\nyou would want to borrow.\n"},{"id":"308403","messageId":"xmqqa8bhb32x.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-15-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:10:30Z","receivedAt":"2016-12-27T19:10:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> +/*\n> + * Signal that the shared index is used by updating its mtime.\n> + *\n> + * This way, shared index can be removed if they have not been used\n> + * for some time. It's ok to fail to update the mtime if we are on a\n> + * read only file system.\n> + */\n> +void freshen_shared_index(char *base_sha1_hex)\n> +{\n> +\tconst char *shared_index = git_path(\"sharedindex.%s\", base_sha1_hex);\n> +\tcheck_and_freshen_file(shared_index, 1);\n\nWhat happens when this call fails? The function returns 0 if the\nfile did not even exist. It also returns 0 if you cannot update its\ntimestamp.\n\nShouldn't the series be exposing freshen_file() instead _and_ taking\nits error return value seriously?\n\n> +}\n> +\n> int read_index_from(struct index_state *istate, const char *path)\n> {\n> \tstruct split_index *split_index;\n> @@ -2273,6 +2286,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,\n> \t\tint ret = write_shared_index(istate, lock, flags);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> +\t} else {\n> +\t\tfreshen_shared_index(sha1_to_hex(si->base_sha1));\n> \t}\n> \n> \treturn write_split_index(istate, lock, flags);\n"},{"id":"308404","messageId":"xmqqvau59ocy.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-7-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 06/21] t1700: add tests for core.splitIndex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:04:31Z","receivedAt":"2016-12-27T19:13:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> +test_expect_success 'set core.splitIndex config variable to true' '\n> +\tgit config core.splitIndex true &&\n> +\t: >three &&\n> +\tgit update-index --add three &&\n> +\tgit ls-files --stage >ls-files.actual &&\n> +\tcat >ls-files.expect < +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tone\n> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tthree\n> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\ttwo\n> +EOF\n> +\ttest_cmp ls-files.expect ls-files.actual &&\n\nIt does not add much value to follow the \"existing\" outdated style\nlike this when you are only adding new tests. Write these like\n\n cat >ls-files.expect <<-\\EOF &&\n\t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tone\n\t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tthree\n\t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\ttwo\n\tEOF\n\nwhich would give incentive to others (or yourself) to update the\nstyle of the existing mess ;-).\n"},{"id":"308405","messageId":"xmqqpokd9ocw.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-9-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:07:37Z","receivedAt":"2016-12-27T19:14:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/git-update-index.txt | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n> index 7386c93162..e091b2a409 100644\n> --- a/Documentation/git-update-index.txt\n> +++ b/Documentation/git-update-index.txt\n> @@ -171,6 +171,12 @@ may not support it yet.\n> \tgiven again, all changes in $GIT_DIR/index are pushed back to\n> \tthe shared index file. This mode is designed for very large\n> \tindexes that take a significant amount of time to read or write.\n> ++\n> +These options take effect whatever the value of the `core.splitIndex`\n> +configuration variable (see linkgit:git-config[1]).\n\nDoesn't the \"whatever...\" clause in this sentence lack its verb\n(presumably, \"is\", right after \"variable\")?\n\n> +emitted when the change goes against the configured value, as the\n> +configured value will take effect next time the index is read and this\n> +will remove the intended effect of the option.\n\nIt feels somewhat strange to see a warning in this case. \n\nIf the user configures the system to usually do X, and issues a\ncommand that explicitly does Y (or \"not-X\"), we do not warn for the\ncommand invocation if it is a single-shot operation. That's the\nusual \"command line overrides configuration\" an end-user expects.\n\nI think you made this deviate from the usual end-user expectation\nbecause it is unpredictable when the configuration kicks in the next\ntime to undo the effect of this command line request. And I agree\nthat it is a very un-nice thing to do to the users if we did not\nwarn here, if we are to keep that unpredictableness.\n\nBut stepping back a bit, we know the user's intention is that with\n\"--split-index\" the user wants to use the split index, even though\nthe user may have configuration not to use the split index. Don't\nwe want to honor that intention? In order to honor that intention\nwithout getting confused by the configured variable to tell us not\nto split, another way is to destroy that unpredictableness. For\nthat, shouldn't we automatically remove the configuration that says\n\"do not split\" (by perhaps set it to \"do split\")? Doing so still\nmay need a warning that says \"we disabled your previously configured\nsetting while at it\", but it would be much better than a warning\nmessage that essentially says \"we do it once, but we will disobey\nyou at an unpredictable time\", I would think.\n"},{"id":"308406","messageId":"xmqqk2al9ocv.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-11-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 10/21] read-cache: regenerate shared index if necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:08:47Z","receivedAt":"2016-12-27T19:14:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> +\tcase 0:\n> +\t\treturn 1; /* 0% means always write a new shared index */\n> +\tcase 100:\n> +\t\treturn 0; /* 100% means never write a new shared index */\n> +\tdefault:\n> +\t\t; /* do nothing: just use the configured value */\n> +\t}\n\nJust like you did in 04/21, write \"break\" to avoid mistakes made in\nthe future, i.e.\n\n\tdefault:\n\t\tbreak; /* just use the configured value */\n\n> +\n> +\t/* Count not shared entries */\n> +\tfor (i = 0; i < istate->cache_nr; i++) {\n> +\t\tstruct cache_entry *ce = istate->cache[i];\n> +\t\tif (!ce->index)\n> +\t\t\tnot_shared++;\n> +\t}\n> +\n> +\treturn istate->cache_nr * max_split < not_shared * 100;\n\nOn a 32-bit arch with 2G int and more than 20 million paths in the\nindex, multiplying by max_split that can come close to 100 can\ntheoretically cause integer overflow, but in practice it probably\ndoes not matter. Or does it?\n"},{"id":"308407","messageId":"xmqqeg0t9oct.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-21-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:11:31Z","receivedAt":"2016-12-27T19:14:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/config.txt | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index e0f5a77980..24e771d22e 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2786,6 +2786,17 @@ splitIndex.maxPercentChange::\n> \tthan 20 percent of the total number of entries.\n> \tSee linkgit:git-update-index[1].\n> \n> +splitIndex.sharedIndexExpire::\n> +\tWhen the split index feature is used, shared index files with\n> +\ta mtime older than this time will be removed when a new shared\n\nAs end-user facing documentation, it would be much better if we can\nrephrase it for those who do not know what a 'mtime' is, and it\nwould be even better if we can do so without losing precision.\n\nI think \"shared index files that were not modified since the time\nthis variable specifies will be removed\" would be understandable and\ncorrect enough?\n\n> +\tindex file is created. The value \"now\" expires all entries\n> +\timmediately, and \"never\" suppresses expiration altogether.\n> +\tThe default value is \"one.week.ago\".\n> +\tNote that each time a new split-index file is created, the\n> +\tmtime of the related shared index file is updated to the\n> +\tcurrent time.\n\nTo match the above suggestion, \"Note that a shared index file is\nconsidered modified (for the purpose of expiration) each time a new\nsplit-index file is created based on it.\"?\n\n> +\tSee linkgit:git-update-index[1].\n> +\n> status.relativePaths::\n> \tBy default, linkgit:git-status[1] shows paths relative to the\n> \tcurrent directory. Setting this variable to `false` shows paths\n"},{"id":"308408","messageId":"xmqq8tr19ocs.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"20161226102222.17150-22-chriscool@tuxfamily.org","subject":"Re: [PATCH v3 21/21] Documentation/git-update-index: explain splitIndex.*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-27T19:13:02Z","receivedAt":"2016-12-27T19:14:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> --split-index::\n> --no-split-index::\n> -\tEnable or disable split index mode. If enabled, the index is\n> -\tsplit into two files, $GIT_DIR/index and $GIT_DIR/sharedindex..\n> -\tChanges are accumulated in $GIT_DIR/index while the shared\n> -\tindex file contains all index entries stays unchanged. If\n> -\tsplit-index mode is already enabled and `--split-index` is\n> -\tgiven again, all changes in $GIT_DIR/index are pushed back to\n> -\tthe shared index file. This mode is designed for very large\n> -\tindexes that take a significant amount of time to read or write.\n> +\tEnable or disable split index mode. If split-index mode is\n> +\talready enabled and `--split-index` is given again, all\n> +\tchanges in $GIT_DIR/index are pushed back to the shared index\n> +\tfile.\n\nIn the world after this series that introduced the percentage-based\nauto consolidation, it smells strange, or even illogical, that index\nis un-split after doing this:\n\n $ git update-index --split-index\n $ git update-index --split-index\n\nBefore this series, it may have been a quick and dirty way to force\nconsolidating the split index without totally disabling the feature,\ni.e. it would have looked more like\n\n $ git update-index --split-index\n ... work work work to accumulate more modifications\n ... consolidate into one --- there was no other way than\n ... disabling it temporarily\n $ git update-index --no-split-index\n ... but the user likes the feature so re-enable it.\n $ git update-index --split-index\n\nwhich I guess is where this strange behaviour comes from.\n\nIt may be something we need to fix to unconfuse the end-users after\nthis series lands. Even though \"first disable and then re-enable\"\ntakes two commands (as opposed to one), it is far more logical. And\nthe percentage-based auto consolidation feature is meant to reduce\nthe need for manual consolidation, so it probably makes sense to\nremove this illogical feature.\n\n> @@ -394,6 +390,31 @@ Although this bit looks similar to assume-unchanged bit, its goal is\n> different from assume-unchanged bit's. Skip-worktree also takes\n> precedence over assume-unchanged bit when both are set.\n> \n> +Split index\n> +-----------\n> +\n> +This mode is designed for very large indexes that take a significant\n> +amount of time to read or write.\n\nThis is not a new problem, but it probably is incorrect to say \"to\nread or write\". It saves time by not rewriting the whole thing but\ninstead write out only the updated bits. You'd still read the whole\nthing while populating the in-core index from the disk, and if\nanything, you'd probably spend _more_ cycles because you'd essentially\nbe reading the base and then reading the delta to apply on top.\n\n> +To avoid deleting a shared index file that is still used, its mtime is\n> +updated to the current time everytime a new split index based on the\n> +shared index file is either created or read from.\n\n\nThe same comment on the mention of \"mtime\" in another patch applies\nhere as well.\n\n"},{"id":"308607","messageId":"CAP8UFD1TwVvsvuffyHuzse_9afbNvSEJtyQyWzn6Rc4KwJNwHQ@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqvau59ocy.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 06/21] t1700: add tests for core.splitIndex","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T08:29:21Z","receivedAt":"2017-01-02T08:29:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:04 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> +test_expect_success 'set core.splitIndex config variable to true' '\n>> + git config core.splitIndex true &&\n>> + : >three &&\n>> + git update-index --add three &&\n>> + git ls-files --stage >ls-files.actual &&\n>> + cat >ls-files.expect <> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one\n>> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 three\n>> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two\n>> +EOF\n>> + test_cmp ls-files.expect ls-files.actual &&\n>\n> It does not add much value to follow the \"existing\" outdated style\n> like this when you are only adding new tests. Write these like\n>\n> cat >ls-files.expect <<-\\EOF &&\n> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 one\n> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 three\n> 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 two\n> EOF\n>\n> which would give incentive to others (or yourself) to update the\n> style of the existing mess ;-).\n\nOk, I will add a patch to update the style of the existing tests at\nthe beginning of the series and then use the same new style in the\ntests I add in later patches.\n"},{"id":"308609","messageId":"CAP8UFD06mxGiZmr4Lwv3M8CedBZaamswzz-Q+mOxuuUFet8KNQ@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqpokd9ocw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T09:39:06Z","receivedAt":"2017-01-02T09:39:17Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:07 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Signed-off-by: Christian Couder \n>> ---\n>> Documentation/git-update-index.txt | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\n>> index 7386c93162..e091b2a409 100644\n>> --- a/Documentation/git-update-index.txt\n>> +++ b/Documentation/git-update-index.txt\n>> @@ -171,6 +171,12 @@ may not support it yet.\n>> given again, all changes in $GIT_DIR/index are pushed back to\n>> the shared index file. This mode is designed for very large\n>> indexes that take a significant amount of time to read or write.\n>> ++\n>> +These options take effect whatever the value of the `core.splitIndex`\n>> +configuration variable (see linkgit:git-config[1]).\n>\n> Doesn't the \"whatever...\" clause in this sentence lack its verb\n> (presumably, \"is\", right after \"variable\")?\n\nI think that it is ok to have no verb here. My preferred online\ndictionary (http://www.wordreference.com/enfr/whatever) gives \"We'll\ntry to free the hostage whatever the cost.\" as an example with no\nverb, though I am not a native speaker.\n\nAlso note that I mostly copied what I had already written for\n--(no-)untracked-cache:\n\n--untracked-cache::\n--no-untracked-cache::\n Enable or disable untracked cache feature. Please use\n `--test-untracked-cache` before enabling it.\n+\nThese options take effect whatever the value of the `core.untrackedCache`\nconfiguration variable (see linkgit:git-config[1]). But a warning is\nemitted when the change goes against the configured value, as the\nconfigured value will take effect next time the index is read and this\nwill remove the intended effect of the option.\n\n>> +emitted when the change goes against the configured value, as the\n>> +configured value will take effect next time the index is read and this\n>> +will remove the intended effect of the option.\n>\n> It feels somewhat strange to see a warning in this case.\n\nWe already discussed the warning issue for --(no-)untracked-cache, and\nI am just following the conclusions of that previous discussion about\n--(no-)untracked-cache. See the end of this message where you\nsuggested the warning:\n\nhttps://public-inbox.org/git/xmqqlh8cg9y2.fsf@gitster.mtv.corp.google.com/\n\n> If the user configures the system to usually do X, and issues a\n> command that explicitly does Y (or \"not-X\"), we do not warn for the\n> command invocation if it is a single-shot operation. That's the\n> usual \"command line overrides configuration\" an end-user expects.\n>\n> I think you made this deviate from the usual end-user expectation\n> because it is unpredictable when the configuration kicks in the next\n> time to undo the effect of this command line request. And I agree\n> that it is a very un-nice thing to do to the users if we did not\n> warn here, if we are to keep that unpredictableness.\n\nI also think it would be strange and user unfriendly for\n--untracked-cache and --split-index to behave differently, and I am\nreluctant at this point to rework the way it works for\n--untracked-cache.\n\n> But stepping back a bit, we know the user's intention is that with\n> \"--split-index\" the user wants to use the split index, even though\n> the user may have configuration not to use the split index. Don't\n> we want to honor that intention? In order to honor that intention\n> without getting confused by the configured variable to tell us not\n> to split, another way is to destroy that unpredictableness. For\n> that, shouldn't we automatically remove the configuration that says\n> \"do not split\" (by perhaps set it to \"do split\")? Doing so still\n> may need a warning that says \"we disabled your previously configured\n> setting while at it\", but it would be much better than a warning\n> message that essentially says \"we do it once, but we will disobey\n> you at an unpredictable time\", I would think.\n\nI wanted to do that for --untracked-cache around one year ago but in\nthe following message:\n\nhttps://public-inbox.org/git/xmqqsi3ckadi.fsf@gitster.mtv.corp.google.com/\n\nyou wrote the following:\n\n\"Why is it a good idea to allow an explicit operation from the\ncommand line to muck with the configuration? If the user wants to\nchange the configuration permanently, \"git config\" has been there\nfor the purpose for all the configuration variables to do so for\nalmost forever. Why is it a good idea to make this variable so\nspecial that the user does not have to use \"git config\" but disable\nor enable it in some other way?\"\n\nIt feels strange that when I do things one way, you suggest another\nway, and the next time in a similar situation when I do things the way\nyou suggested previously, then you suggest the way I did it initially\nthe first time...\n"},{"id":"308618","messageId":"CAP8UFD3_1EN=0EsD12Cew1MuW8yhtPAZw0M_g3wmvKFk-uGXxw@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqk2al9ocv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 10/21] read-cache: regenerate shared index if necessary","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T11:23:24Z","receivedAt":"2017-01-02T11:23:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:08 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> + case 0:\n>> + return 1; /* 0% means always write a new shared index */\n>> + case 100:\n>> + return 0; /* 100% means never write a new shared index */\n>> + default:\n>> + ; /* do nothing: just use the configured value */\n>> + }\n>\n> Just like you did in 04/21, write \"break\" to avoid mistakes made in\n> the future, i.e.\n>\n> default:\n> break; /* just use the configured value */\n\nOk, I will do that.\n\n>> +\n>> + /* Count not shared entries */\n>> + for (i = 0; i < istate->cache_nr; i++) {\n>> + struct cache_entry *ce = istate->cache[i];\n>> + if (!ce->index)\n>> + not_shared++;\n>> + }\n>> +\n>> + return istate->cache_nr * max_split < not_shared * 100;\n>\n> On a 32-bit arch with 2G int and more than 20 million paths in the\n> index, multiplying by max_split that can come close to 100 can\n> theoretically cause integer overflow, but in practice it probably\n> does not matter. Or does it?\n\nFrom a cursory look a \"struct cache_entry\" takes at least 80 bytes\nwithout counting the \"char name[FLEX_ARRAY]\" on a 32 bit machine, so I\ndon't think it would be a good idea to work on a repo with 20 million\npaths on a 32 bit machine, but maybe theoretically it could be a\nproblem.\n\nTo be safe I think I will use:\n\nreturn (int64_t)istate->cache_nr * max_split < (int64_t)not_shared * 100;\n"},{"id":"308621","messageId":"CAP8UFD1wmbR_rHyqn0q=0hw6-hHYFTzr=3yxS2XS9qTdY1kWFA@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqlgv1b34y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T13:50:18Z","receivedAt":"2017-01-02T13:50:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:09 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Signed-off-by: Christian Couder \n>> ---\n>> Documentation/config.txt | 13 +++++++++++++\n>> 1 file changed, 13 insertions(+)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 221c5982c0..e0f5a77980 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2773,6 +2773,19 @@ showbranch.default::\n>> The default set of branches for linkgit:git-show-branch[1].\n>> See linkgit:git-show-branch[1].\n>>\n>> +splitIndex.maxPercentChange::\n>> + When the split index feature is used, this specifies the\n>> + percent of entries the split index can contain compared to the\n>> + whole number of entries in both the split index and the shared\n>\n> s/whole/total/ to match the last sentence of this section, perhaps?\n> \"The number of all entries\" would also be OK, but \"the whole number\n> of entries\" sounds a bit strange.\n\nYeah \"total\" is better than \"whole\" here. I will use \"total\".\n"},{"id":"308622","messageId":"CAP8UFD1EgOxcPi=tpiosKkYMcCZe+b6gwW0CKt2sE1NZ7gQv=A@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqa8bhb32x.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T14:09:12Z","receivedAt":"2017-01-02T14:09:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:10 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> +/*\n>> + * Signal that the shared index is used by updating its mtime.\n>> + *\n>> + * This way, shared index can be removed if they have not been used\n>> + * for some time. It's ok to fail to update the mtime if we are on a\n>> + * read only file system.\n>> + */\n>> +void freshen_shared_index(char *base_sha1_hex)\n>> +{\n>> + const char *shared_index = git_path(\"sharedindex.%s\", base_sha1_hex);\n>> + check_and_freshen_file(shared_index, 1);\n>\n> What happens when this call fails? The function returns 0 if the\n> file did not even exist. It also returns 0 if you cannot update its\n> timestamp.\n\nYeah and I don't think it's a problem in either case.\nIf we cannot update its timestamp, it's not a problem, as we could be\non a read-only file system, and you said in a previous iteration that\nwe should not even warn in this case.\nIf the file does not exist, it could be because it has just been\ndeleted for a good reason, and anyway, if it is a problem that the\nshared index file has been deleted, it is better addressed when we\nwill actually need the shared index file to read something into from\nit, rather than just to update its mtime.\n\n> Shouldn't the series be exposing freshen_file() instead _and_ taking\n> its error return value seriously?\n\nSo what should we do if freshen_file() returns 0 which means that the\nfreshening failed?\n\n>> +}\n"},{"id":"308623","messageId":"CAP8UFD0bgxVrc=RGHs1GrZ_5PF4cdfhqXLMiCSJTNw9axrr=_w@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqeg0t9oct.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T14:33:29Z","receivedAt":"2017-01-02T14:33:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:11 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Signed-off-by: Christian Couder \n>> ---\n>> Documentation/config.txt | 11 +++++++++++\n>> 1 file changed, 11 insertions(+)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index e0f5a77980..24e771d22e 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2786,6 +2786,17 @@ splitIndex.maxPercentChange::\n>> than 20 percent of the total number of entries.\n>> See linkgit:git-update-index[1].\n>>\n>> +splitIndex.sharedIndexExpire::\n>> + When the split index feature is used, shared index files with\n>> + a mtime older than this time will be removed when a new shared\n>\n> As end-user facing documentation, it would be much better if we can\n> rephrase it for those who do not know what a 'mtime' is, and it\n> would be even better if we can do so without losing precision.\n>\n> I think \"shared index files that were not modified since the time\n> this variable specifies will be removed\" would be understandable and\n> correct enough?\n\nYeah, I agree it is better for end users. I will use what you suggest.\n\n>> + index file is created. The value \"now\" expires all entries\n>> + immediately, and \"never\" suppresses expiration altogether.\n>> + The default value is \"one.week.ago\".\n>> + Note that each time a new split-index file is created, the\n>> + mtime of the related shared index file is updated to the\n>> + current time.\n>\n> To match the above suggestion, \"Note that a shared index file is\n> considered modified (for the purpose of expiration) each time a new\n> split-index file is created based on it.\"?\n\nYeah, I also agree it is better and will use that.\n"},{"id":"308673","messageId":"CAP8UFD2qK5uv5mrCOe1h48H8sUezC_Jk3-fftBxdOhBssmxTgA@mail.gmail.com","threadId":"44768","inReplyTo":"xmqq8tr19ocs.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 21/21] Documentation/git-update-index: explain splitIndex.*","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-02T16:04:11Z","receivedAt":"2017-01-02T16:04:23Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Dec 27, 2016 at 8:13 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> --split-index::\n>> --no-split-index::\n>> - Enable or disable split index mode. If enabled, the index is\n>> - split into two files, $GIT_DIR/index and $GIT_DIR/sharedindex..\n>> - Changes are accumulated in $GIT_DIR/index while the shared\n>> - index file contains all index entries stays unchanged. If\n>> - split-index mode is already enabled and `--split-index` is\n>> - given again, all changes in $GIT_DIR/index are pushed back to\n>> - the shared index file. This mode is designed for very large\n>> - indexes that take a significant amount of time to read or write.\n>> + Enable or disable split index mode. If split-index mode is\n>> + already enabled and `--split-index` is given again, all\n>> + changes in $GIT_DIR/index are pushed back to the shared index\n>> + file.\n>\n> In the world after this series that introduced the percentage-based\n> auto consolidation, it smells strange, or even illogical, that index\n> is un-split after doing this:\n>\n> $ git update-index --split-index\n> $ git update-index --split-index\n>\n> Before this series, it may have been a quick and dirty way to force\n> consolidating the split index without totally disabling the feature,\n> i.e. it would have looked more like\n>\n> $ git update-index --split-index\n> ... work work work to accumulate more modifications\n> ... consolidate into one --- there was no other way than\n> ... disabling it temporarily\n> $ git update-index --no-split-index\n> ... but the user likes the feature so re-enable it.\n> $ git update-index --split-index\n>\n> which I guess is where this strange behaviour comes from.\n>\n> It may be something we need to fix to unconfuse the end-users after\n> this series lands. Even though \"first disable and then re-enable\"\n> takes two commands (as opposed to one), it is far more logical. And\n> the percentage-based auto consolidation feature is meant to reduce\n> the need for manual consolidation, so it probably makes sense to\n> remove this illogical feature.\n\nYeah, I tend to agree that this feature could be removed later. Though\nbefore removing it, I'd like to hear Duy's opinion on this as he\ncreated the feature in the first place.\n\n>> @@ -394,6 +390,31 @@ Although this bit looks similar to assume-unchanged bit, its goal is\n>> different from assume-unchanged bit's. Skip-worktree also takes\n>> precedence over assume-unchanged bit when both are set.\n>>\n>> +Split index\n>> +-----------\n>> +\n>> +This mode is designed for very large indexes that take a significant\n>> +amount of time to read or write.\n>\n> This is not a new problem, but it probably is incorrect to say \"to\n> read or write\". It saves time by not rewriting the whole thing but\n> instead write out only the updated bits. You'd still read the whole\n> thing while populating the in-core index from the disk, and if\n> anything, you'd probably spend _more_ cycles because you'd essentially\n> be reading the base and then reading the delta to apply on top.\n\nOk, then what about:\n\n+This mode is designed for repositories with very large indexes, and aims\n+at reducing the time it takes to repeatedly write these indexes.\n\n>> +To avoid deleting a shared index file that is still used, its mtime is\n>> +updated to the current time everytime a new split index based on the\n>> +shared index file is either created or read from.\n>\n> The same comment on the mention of \"mtime\" in another patch applies\n> here as well.\n\nOk, I will use \"modification time\" instead of \"mtime\".\n\nThanks.\n"},{"id":"308702","messageId":"xmqq8tqs47wd.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD1TwVvsvuffyHuzse_9afbNvSEJtyQyWzn6Rc4KwJNwHQ@mail.gmail.com","subject":"Re: [PATCH v3 06/21] t1700: add tests for core.splitIndex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-03T12:58:42Z","receivedAt":"2017-01-03T12:58:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Ok, I will add a patch to update the style of the existing tests at\n> the beginning of the series and then use the same new style in the\n> tests I add in later patches.\n\nThat's not what I meant---I was expecting and was willing to accept\na corrected patch that leaves existing old-fashioned ones as they\nare while making sure that added ones are modern, to reduce the cost\nof finishing this series, leaving the style fixes of existing ones\nfor future clean-up task that can be done by anybody after the dust\nfrom this series settles.\n\nA preparatory clean-up patch before the series like you plan is fine\n(eh, rather, \"extra nice\"), so... thanks.\n"},{"id":"308932","messageId":"xmqqshou35zr.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD06mxGiZmr4Lwv3M8CedBZaamswzz-Q+mOxuuUFet8KNQ@mail.gmail.com","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-07T21:38:48Z","receivedAt":"2017-01-07T21:38:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> It feels strange that when I do things one way, you suggest another\n> way, and the next time in a similar situation when I do things the way\n> you suggested previously, then you suggest the way I did it initially\n> the first time...\n\nPerhaps because neither is quite satisfactory and I am being forced\nto choose between the two unsatifactory designs? In any case, I\nmostly am and was pointing out the issues and not saying the other\none is the most preferred solution in these threads.\n\nI think there should just be one authoritative source of the truth,\nand I have always thought it should be the bit set in the index file\nwhen the command line option is used, because that was the way the\nfeature was introduced first and I am superstitious about end-user\ninertia. And from that point of view, no matter how you make this\nnew \"config\" thing interact with it, it would always give a strange\nand unsatifactory end-user experience, at least to me.\n\nPerhaps we should declare that config will be the one and only way\nin the future and start deprecating the command line option way.\nThat will remove the need for two to interact with each other.\n\nI dunno.\n"},{"id":"308934","messageId":"xmqqo9zi35n6.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD1EgOxcPi=tpiosKkYMcCZe+b6gwW0CKt2sE1NZ7gQv=A@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-07T21:46:21Z","receivedAt":"2017-01-07T21:46:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> So what should we do if freshen_file() returns 0 which means that the\n> freshening failed?\n\nYou tell me ;-) as you are the one who is proposing this feature.\n\nIsn't a failure to freshen it a grave error? We are letting a\nbase/shared index file that is known to be in-use go stale and\neventually subject for garbage collection, and the user should be\nnotified in some way before the actual GC happens that renders the\nindex file unusable?\n\nWhat is the failure mode after such a premature GC happens? What\ndoes the end-user see? Can you try to (1) split the index (2)\nmodify bunch of entries (3) remove the base/shared index with /bin/rm\nand then see how various Git commands fail? Do they fail gracefully?\n\nI am trying to gauge the seriousness of ignoring such an error here.\n"},{"id":"309021","messageId":"CACsJy8AR6yNr0y+_JZDkW-HO_yHPkUx_6zbLGoviKQBOVcSg5A@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqo9zi35n6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-01-09T10:55:52Z","receivedAt":"2017-01-09T10:57:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> So what should we do if freshen_file() returns 0 which means that the\n>> freshening failed?\n>\n> You tell me ;-) as you are the one who is proposing this feature.\n\nMy answer is, we are not worse than freshening loose objects case\n(especially since I took the idea from there). In both cases we\nsilently ignore utime() error (sha1_file.c does retry, but will do\nnothing else if the retry fails). And errors in odb are much more\nserious than index files. If we are to improve it, I think we should\ndo it inside check_and_freshen_file(), maybe with an optional flag to\nsilence it.\n\n> Isn't a failure to freshen it a grave error? We are letting a\n> base/shared index file that is known to be in-use go stale and\n> eventually subject for garbage collection, and the user should be\n> notified in some way before the actual GC happens that renders the\n> index file unusable?\n>\n> What is the failure mode after such a premature GC happens? What\n> does the end-user see? Can you try to (1) split the index (2)\n> modify bunch of entries (3) remove the base/shared index with /bin/rm\n> and then see how various Git commands fail? Do they fail gracefully?\n>\n> I am trying to gauge the seriousness of ignoring such an error here.\n\nIf we fail to refresh it and the file is old enough and gc happens,\nany index file referenced to it are broken. Any commands that read the\nindex will die(). The best you could do is delete $GIT_DIR/index and\nread-tree HEAD.\n-- \nDuy\n"},{"id":"309023","messageId":"CACsJy8D1Pf6zTS8gqv5Gq6xMxNNbDrTpKHGADRMKNqW1FAzZvA@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqshou35zr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-01-09T11:18:36Z","receivedAt":"2017-01-09T11:19:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jan 8, 2017 at 4:38 AM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> It feels strange that when I do things one way, you suggest another\n>> way, and the next time in a similar situation when I do things the way\n>> you suggested previously, then you suggest the way I did it initially\n>> the first time...\n>\n> Perhaps because neither is quite satisfactory and I am being forced\n> to choose between the two unsatifactory designs? In any case, I\n> mostly am and was pointing out the issues and not saying the other\n> one is the most preferred solution in these threads.\n>\n> I think there should just be one authoritative source of the truth,\n\nEither that, or we make sure all sources of truth are consistent. In\nthis case, 'update --split-index' could update core.splitIndex if it\nfinds that the config tells a different story. As a plumbing though, I\nrather leave update-index do simple things, even if it means the user\nhas to clean up after it (or before it) with \"git config -unset\ncore.splitIndex\". Another option is refuse to execute --split-index in\nthe presence of (conflicting) core.splitIndex. We kind of force the\nuser to keep all sources of truth consistent this way while leaving a\nback door (\"git -c core.splitIndex= update-index\") for those who need\ntools to recover from a bad case.\n\n> and I have always thought it should be the bit set in the index file\n> when the command line option is used, because that was the way the\n> feature was introduced first and I am superstitious about end-user\n> inertia. And from that point of view, no matter how you make this\n> new \"config\" thing interact with it, it would always give a strange\n> and unsatifactory end-user experience, at least to me.\n>\n> Perhaps we should declare that config will be the one and only way\n> in the future and start deprecating the command line option way.\n> That will remove the need for two to interact with each other.\n>\n> I dunno.\n\n\n\n-- \nDuy\n"},{"id":"309036","messageId":"xmqqvatouwsh.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CACsJy8AR6yNr0y+_JZDkW-HO_yHPkUx_6zbLGoviKQBOVcSg5A@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-09T14:34:38Z","receivedAt":"2017-01-09T14:36:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n>> Christian Couder writes:\n>>\n>>> So what should we do if freshen_file() returns 0 which means that the\n>>> freshening failed?\n>>\n>> You tell me ;-) as you are the one who is proposing this feature.\n>\n> My answer is, we are not worse than freshening loose objects case\n> (especially since I took the idea from there). \n\nI do not think so, unfortunately. Loose object files with stale\ntimestamps are not removed as long as objects are still reachable.\nFor the base/shared index file, the timestamp is the only thing that\nprotects them from pruning, unless it is serving as the base file\nfor the currently active $GIT_DIR/index that is split.\n\n>> What is the failure mode after such a premature GC happens? What\n>> does the end-user see? Can you try to (1) split the index (2)\n>> modify bunch of entries (3) remove the base/shared index with /bin/rm\n>> and then see how various Git commands fail? Do they fail gracefully?\n>>\n>> I am trying to gauge the seriousness of ignoring such an error here.\n>\n> If we fail to refresh it and the file is old enough and gc happens,\n> any index file referenced to it are broken. Any commands that read the\n> index will die(). The best you could do is delete $GIT_DIR/index and\n> read-tree HEAD.\n"},{"id":"309723","messageId":"CACsJy8B_xe6RtszPrncvDdSYosNUQxvhcEQ3DO_WO0sepXqvvQ@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqvatouwsh.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-01-19T12:13:04Z","receivedAt":"2017-01-19T12:13:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n>>> Christian Couder writes:\n>>>\n>>>> So what should we do if freshen_file() returns 0 which means that the\n>>>> freshening failed?\n>>>\n>>> You tell me ;-) as you are the one who is proposing this feature.\n>>\n>> My answer is, we are not worse than freshening loose objects case\n>> (especially since I took the idea from there).\n>\n> I do not think so, unfortunately. Loose object files with stale\n> timestamps are not removed as long as objects are still reachable.\n\nBut there are plenty of unreachable loose objects, added in index,\nthen got replaced with new versions. cache-tree can create loose trees\ntoo and it's been run more often, behind user's back, to take\nadvantage of the shortcut in unpack-trees.\n\n> For the base/shared index file, the timestamp is the only thing that\n> protects them from pruning, unless it is serving as the base file\n> for the currently active $GIT_DIR/index that is split.\n-- \nDuy\n"},{"id":"309758","messageId":"xmqqlgu627uj.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CACsJy8B_xe6RtszPrncvDdSYosNUQxvhcEQ3DO_WO0sepXqvvQ@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-19T19:00:20Z","receivedAt":"2017-01-19T19:16:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano wrote:\n>> Duy Nguyen writes:\n>>\n>>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n>>>> Christian Couder writes:\n>>>>\n>>>>> So what should we do if freshen_file() returns 0 which means that the\n>>>>> freshening failed?\n>>>>\n>>>> You tell me ;-) as you are the one who is proposing this feature.\n>>>\n>>> My answer is, we are not worse than freshening loose objects case\n>>> (especially since I took the idea from there).\n>>\n>> I do not think so, unfortunately. Loose object files with stale\n>> timestamps are not removed as long as objects are still reachable.\n>\n> But there are plenty of unreachable loose objects, added in index,\n> then got replaced with new versions. cache-tree can create loose trees\n> too and it's been run more often, behind user's back, to take\n> advantage of the shortcut in unpack-trees.\n\nI am not sure if I follow. Aren't objects reachable from the\ncache-tree in the index protected from gc?\n\nNot that I think freshening would actually fail in a repository\nwhere you can actually write into to update the index or its refs to\nmake a difference (iow, even if we make it die() loudly when shared\nindex cannot be \"touched\" because we are paranoid, no real life\nusage will trigger that die(), and if a repository does trigger the\ndie(), I think you would really want to know about it).\n"},{"id":"309815","messageId":"CACsJy8DEZ1eOe5WXbS8WcTSdghadwHS4X7VDL97XvkVrfDrayQ@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqlgu627uj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-01-20T10:44:41Z","receivedAt":"2017-01-20T10:46:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jan 20, 2017 at 2:00 AM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano wrote:\n>>> Duy Nguyen writes:\n>>>\n>>>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n>>>>> Christian Couder writes:\n>>>>>\n>>>>>> So what should we do if freshen_file() returns 0 which means that the\n>>>>>> freshening failed?\n>>>>>\n>>>>> You tell me ;-) as you are the one who is proposing this feature.\n>>>>\n>>>> My answer is, we are not worse than freshening loose objects case\n>>>> (especially since I took the idea from there).\n>>>\n>>> I do not think so, unfortunately. Loose object files with stale\n>>> timestamps are not removed as long as objects are still reachable.\n>>\n>> But there are plenty of unreachable loose objects, added in index,\n>> then got replaced with new versions. cache-tree can create loose trees\n>> too and it's been run more often, behind user's back, to take\n>> advantage of the shortcut in unpack-trees.\n>\n> I am not sure if I follow. Aren't objects reachable from the\n> cache-tree in the index protected from gc?\n\nI think the problem is loose objects created then gc run just before\nthey are referenced (e.g. index written down). But I think I may be\nfollowing a wrong road. If mtime is in fact to deal with race\nconditions, applying the same idea here is wrong because we have a\ndifferent problem here.\n\n> Not that I think freshening would actually fail in a repository\n> where you can actually write into to update the index or its refs to\n> make a difference (iow, even if we make it die() loudly when shared\n> index cannot be \"touched\" because we are paranoid, no real life\n> usage will trigger that die(), and if a repository does trigger the\n> die(), I think you would really want to know about it).\n-- \nDuy\n"},{"id":"309979","messageId":"CAP8UFD2aQJ92KjzTQTGLyYeEuVk9TK51mn05OSWCZu5c4c6WuQ@mail.gmail.com","threadId":"44768","inReplyTo":"CACsJy8D1Pf6zTS8gqv5Gq6xMxNNbDrTpKHGADRMKNqW1FAzZvA@mail.gmail.com","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-23T15:55:38Z","receivedAt":"2017-01-23T15:55:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jan 9, 2017 at 12:18 PM, Duy Nguyen wrote:\n> On Sun, Jan 8, 2017 at 4:38 AM, Junio C Hamano wrote:\n>> Christian Couder writes:\n>>\n>>> It feels strange that when I do things one way, you suggest another\n>>> way, and the next time in a similar situation when I do things the way\n>>> you suggested previously, then you suggest the way I did it initially\n>>> the first time...\n>>\n>> Perhaps because neither is quite satisfactory and I am being forced\n>> to choose between the two unsatifactory designs? In any case, I\n>> mostly am and was pointing out the issues and not saying the other\n>> one is the most preferred solution in these threads.\n>>\n>> I think there should just be one authoritative source of the truth,\n>\n> Either that, or we make sure all sources of truth are consistent. In\n> this case, 'update --split-index' could update core.splitIndex if it\n> finds that the config tells a different story. As a plumbing though, I\n> rather leave update-index do simple things, even if it means the user\n> has to clean up after it (or before it) with \"git config -unset\n> core.splitIndex\". Another option is refuse to execute --split-index in\n> the presence of (conflicting) core.splitIndex. We kind of force the\n> user to keep all sources of truth consistent this way while leaving a\n> back door (\"git -c core.splitIndex= update-index\") for those who need\n> tools to recover from a bad case.\n>\n>> and I have always thought it should be the bit set in the index file\n>> when the command line option is used, because that was the way the\n>> feature was introduced first and I am superstitious about end-user\n>> inertia. And from that point of view, no matter how you make this\n>> new \"config\" thing interact with it, it would always give a strange\n>> and unsatifactory end-user experience, at least to me.\n>>\n>> Perhaps we should declare that config will be the one and only way\n>> in the future and start deprecating the command line option way.\n>> That will remove the need for two to interact with each other.\n\nThat would be my preferred solution as I think it is the simplest in\nthe end for users.\nAlso, as Duy wrote above, one can always use something like \"git -c\ncore.splitIndex= ...\", which by the way can work for any command, not\njust \"update-index\".\n\nAnyway we would have to introduce core.splitIndex first, and it\nwouldn't make sense to have a different behavior between\n--[no-]split-index and --[no-]untracked-cache in the meantime before\nthey are deprecated and eventually removed.\n\nSo let's just go with the implementation in this series, which is\nsimilar to --[no-]untracked-cache, and let's plan to deprecate\n--[no-]split-index and --[no-]untracked-cache in a later patch series.\n"},{"id":"309989","messageId":"CAP8UFD2LWGtNtdhtQTZXqsACBvK=jD25vt8M4HzBRBVS1sJ+=Q@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqlgu627uj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-23T18:14:11Z","receivedAt":"2017-01-23T18:14:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Jan 19, 2017 at 8:00 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano wrote:\n>>> Duy Nguyen writes:\n>>>\n>>>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano wrote:\n>>>>> Christian Couder writes:\n>>>>>\n>>>>>> So what should we do if freshen_file() returns 0 which means that the\n>>>>>> freshening failed?\n>>>>>\n>>>>> You tell me ;-) as you are the one who is proposing this feature.\n\nBefore the above question I already had given my opinion about what we\nshould do.\n\nThere are the following cases:\n\n- Freshening failed because the shared index file does not exist\nanymore. In this case it could have been removed for a good reason\n(for example maybe the user wants to remove all the shared index\nfiles), or it could be a bug somewhere else. Anyway we cannot know and\nthe user will get an error if the shared index file that disappeared\nis read from afterwards, so I don't think we need to warn or do\nanything.\n\n- Freshening failed because the mtime of the shared index cannot be\nchanged. You already in a previous email wrote that we shoudn't warn\nif the file system is read only, and I agree with that, as anyway if\nthe file system is read only, the shared index file cannot be deleted,\nso there is no risk from the current user. Also the split index mode\nis useful to speed up index writing at the cost of making index\nreading a little slower, so its use in a read only mode should not be\nthe primary way it is used.\n\nSo my opinion is that there are good reasons not to do anything if\nfreshening fails.\n\n>>>> My answer is, we are not worse than freshening loose objects case\n>>>> (especially since I took the idea from there).\n>>>\n>>> I do not think so, unfortunately. Loose object files with stale\n>>> timestamps are not removed as long as objects are still reachable.\n\nAs the current index is read, which freshens its shared index file,\nbefore a new index is created, the number of shared index files\ndoesn't go below 2. This can be seen in the tests changed in patch\n19/21. So the risk of deleting interesting shared index files is quite\nlow in my opinion.\n\nAlso in general the split-index mode is useful when you often write\nnew indexes, and in this case shared index files that are used will\noften be freshened, so the risk of deleting interesting shared index\nfiles should be low.\n\n>> But there are plenty of unreachable loose objects, added in index,\n>> then got replaced with new versions. cache-tree can create loose trees\n>> too and it's been run more often, behind user's back, to take\n>> advantage of the shortcut in unpack-trees.\n>\n> I am not sure if I follow. Aren't objects reachable from the\n> cache-tree in the index protected from gc?\n>\n> Not that I think freshening would actually fail in a repository\n> where you can actually write into to update the index or its refs to\n> make a difference (iow, even if we make it die() loudly when shared\n> index cannot be \"touched\" because we are paranoid, no real life\n> usage will trigger that die(), and if a repository does trigger the\n> die(), I think you would really want to know about it).\n\nAs I wrote above, I think if we can actually write the shared index\nfile but its freshening fails, it probably means that the shared index\nfile has been removed behind us, and this case is equivalent as when\nloose files have been removed behind us.\n"},{"id":"309999","messageId":"xmqqefztsj4c.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD2LWGtNtdhtQTZXqsACBvK=jD25vt8M4HzBRBVS1sJ+=Q@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-23T18:53:39Z","receivedAt":"2017-01-23T18:53:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Also in general the split-index mode is useful when you often write\n> new indexes, and in this case shared index files that are used will\n> often be freshened, so the risk of deleting interesting shared index\n> files should be low.\n> ...\n>> Not that I think freshening would actually fail in a repository\n>> where you can actually write into to update the index or its refs to\n>> make a difference (iow, even if we make it die() loudly when shared\n>> index cannot be \"touched\" because we are paranoid, no real life\n>> usage will trigger that die(), and if a repository does trigger the\n>> die(), I think you would really want to know about it).\n>\n> As I wrote above, I think if we can actually write the shared index\n> file but its freshening fails, it probably means that the shared index\n> file has been removed behind us, and this case is equivalent as when\n> loose files have been removed behind us.\n\nOK, so it is unlikely to happen, and when it happens it leads to a\ncatastrophic failure---do we just ignore or do we report an error?\n"},{"id":"310005","messageId":"xmqqo9yxr1hg.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD2aQJ92KjzTQTGLyYeEuVk9TK51mn05OSWCZu5c4c6WuQ@mail.gmail.com","subject":"Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-23T19:59:55Z","receivedAt":"2017-01-23T20:00:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>>> Perhaps we should declare that config will be the one and only way\n>>> in the future and start deprecating the command line option way.\n>>> That will remove the need for two to interact with each other.\n>\n> That would be my preferred solution as I think it is the simplest in\n> the end for users.\n> Also, as Duy wrote above, one can always use something like \"git -c\n> core.splitIndex= ...\", which by the way can work for any command, not\n> just \"update-index\".\n>\n> Anyway we would have to introduce core.splitIndex first, and it\n> wouldn't make sense to have a different behavior between\n> --[no-]split-index and --[no-]untracked-cache in the meantime before\n> they are deprecated and eventually removed.\n>\n> So let's just go with the implementation in this series, which is\n> similar to --[no-]untracked-cache, and let's plan to deprecate\n> --[no-]split-index and --[no-]untracked-cache in a later patch series.\n\nSounds like we have a plan. Thanks.\n"},{"id":"310161","messageId":"CAP8UFD3KXOgVOhuMtws36XPiek56U4mQKdUs07hzKc-dC=ppmA@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqefztsj4c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-25T14:35:47Z","receivedAt":"2017-01-25T14:35:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jan 23, 2017 at 7:53 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Also in general the split-index mode is useful when you often write\n>> new indexes, and in this case shared index files that are used will\n>> often be freshened, so the risk of deleting interesting shared index\n>> files should be low.\n>> ...\n>>> Not that I think freshening would actually fail in a repository\n>>> where you can actually write into to update the index or its refs to\n>>> make a difference (iow, even if we make it die() loudly when shared\n>>> index cannot be \"touched\" because we are paranoid, no real life\n>>> usage will trigger that die(), and if a repository does trigger the\n>>> die(), I think you would really want to know about it).\n>>\n>> As I wrote above, I think if we can actually write the shared index\n>> file but its freshening fails, it probably means that the shared index\n>> file has been removed behind us, and this case is equivalent as when\n>> loose files have been removed behind us.\n>\n> OK, so it is unlikely to happen, and when it happens it leads to a\n> catastrophic failure---do we just ignore or do we report an error?\n\nWell, when we cannot freshen a loose file (with\nfreshen_loose_object()), we don't warn or die, we just write the loose\nfile. But here we cannot write the shared index file.\n\nAnd this doesn't lead to a catastrophic failure right away. There\ncould be a catastrophic failure if the shared index file is needed\nagain later, but we are not sure that it's going to be needed later.\nIn fact it may have just been removed because it won't be needed\nlater.\n\nSo I am not sure it's a good idea to anticipate a catastrophic failure\nthat may not happen. Perhaps we could just warn, but I am not sure it\nwill help the user. If the catastrophic failure doesn't happen because\nthe shared index file is not needed, I can't see how the warning could\nhelp. And if there is a catastrophic failure, the following will be\ndisplayed:\n\nfatal: .git/sharedindex.cbfe41352a4623d4d3e9757861fed53f3094e0ac:\nindex file open failed: No such file or directory\n\nand I don't see how the warning could help on top of that. It could at\nmost repeat the same thing.\n"},{"id":"310200","messageId":"xmqqsho6amm7.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD3KXOgVOhuMtws36XPiek56U4mQKdUs07hzKc-dC=ppmA@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-25T20:52:16Z","receivedAt":"2017-01-25T20:52:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Well, when we cannot freshen a loose file (with\n> freshen_loose_object()), we don't warn or die, we just write the loose\n> file. But here we cannot write the shared index file.\n\nI think that is an excellent point. Let me make sure I got you\nright. For loose object files, we may attempt to freshen and when\nit fails we stay silent and do not talk about the failure. Instead,\nwe write the same file again. That will have two potential outcomes:\n\n 1. write fails and we fail the whole thing. It is very clear to\n the user that there is something wrong going on.\n\n 2. write succeeds, and because we just wrote it, we know that the\n file is fresh and is protected from gc.\n\nSo the \"freshen and if fails just write\" is sufficient. It is\nabsolutely the right thing to do for loose object files.\n\nWhen we are forking off a new split index file based on an old\nshared index file, we may attempt to \"touch\" the old shared one, but\nwe cannot write the old shared one again, because other split index\nmay be based on that, and we do not have enough information to\nrecreate the old one [*1*]. The fallback safety is not available.\n\n> And this doesn't lead to a catastrophic failure right away. \n\nExactly.\n\n> There\n> could be a catastrophic failure if the shared index file is needed\n> again later, but we are not sure that it's going to be needed later.\n> In fact it may have just been removed because it won't be needed\n> later.\n\nYou are listing only the irrelevant cases. The shared one may be\nused immediately, and the user can keep using it for a while without\n\"touching\". Perhaps the shared one was chowned to \"root\" while the\nuser is looking the other way, and its timestamp is now frozen to\nthe time of chown. It is a ticking time-bomb that will go off when\nyour expiry logic kicks in.\n\n> So I am not sure it's a good idea to anticipate a catastrophic failure\n> that may not happen. Perhaps we could just warn, but I am not sure it\n> will help the user. If the catastrophic failure doesn't happen because\n> the shared index file is not needed, I can't see how the warning could\n> help. And if there is a catastrophic failure, the following will be\n> displayed:\n>\n> fatal: .git/sharedindex.cbfe41352a4623d4d3e9757861fed53f3094e0ac:\n> index file open failed: No such file or directory\n\nThat \"fatal\" is given _ONLY_ after time passes and our failure to\n\"touch\" the file that is still-in-use left it subject to \"gc\". Of\ncourse, when \"fatal\" is given, it is too late to warn about ticking\ntime bomb.\n\nAt the time we notice a ticking time bomb is the only sensible time\nto warn. Or better yet take a corrective action.\n\nAs I expect (and you seem to agree) that a failure to \"touch\" would\nbe a very rare event (like, sysadmin chowning it to \"root\" by\nmistake), I do not mind if the \"corrective action\" were \"immediately\nunsplit the index, so that at least the current and the latest index\ncontents will be written out safely to a new single unsplit index\nfile\". That won't help _other_ split index files that are based on\nthe same \"untouchable\" shared index, but I do not think that is a\nproblem we need to solve---if they are still in use, the code that\nuse them will notice it, and otherwise they are not in use and can\nbe aged away safely.\n\n\n[Footnote]\n\n*1* My understanding is that we lose information on stale entries in\n the shared file that are covered by the split index overlay\n after read_index() returns, so we _might_ be able to write the\n \"old\" one that is sufficient for _our_ split index, but we do\n not have good enough information to recreate \"old\" one usable by\n other split index files.\n"},{"id":"310603","messageId":"CAP8UFD3iWg5g-h209VDyg1U03Jmma8nTONyCYB-kN7GwspkL8Q@mail.gmail.com","threadId":"44768","inReplyTo":"xmqqsho6amm7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-01-31T14:06:22Z","receivedAt":"2017-01-31T14:07:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jan 25, 2017 at 9:52 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> Well, when we cannot freshen a loose file (with\n>> freshen_loose_object()), we don't warn or die, we just write the loose\n>> file. But here we cannot write the shared index file.\n>\n> I think that is an excellent point. Let me make sure I got you\n> right. For loose object files, we may attempt to freshen and when\n> it fails we stay silent and do not talk about the failure. Instead,\n> we write the same file again. That will have two potential outcomes:\n>\n> 1. write fails and we fail the whole thing. It is very clear to\n> the user that there is something wrong going on.\n>\n> 2. write succeeds, and because we just wrote it, we know that the\n> file is fresh and is protected from gc.\n>\n> So the \"freshen and if fails just write\" is sufficient. It is\n> absolutely the right thing to do for loose object files.\n>\n> When we are forking off a new split index file based on an old\n> shared index file, we may attempt to \"touch\" the old shared one, but\n> we cannot write the old shared one again, because other split index\n> may be based on that, and we do not have enough information to\n> recreate the old one [*1*]. The fallback safety is not available.\n\nYeah I agree. But note that I was talking about the case where the\nshared index file does not exist anymore.\nI was just saying that when the shared index file has been removed\nbehind us, it is equivalent as when\nloose files have been removed behind us, except that we cannot write\nthe removed file to disk.\n\n>> And this doesn't lead to a catastrophic failure right away.\n>\n> Exactly.\n>\n>> There\n>> could be a catastrophic failure if the shared index file is needed\n>> again later, but we are not sure that it's going to be needed later.\n>> In fact it may have just been removed because it won't be needed\n>> later.\n>\n> You are listing only the irrelevant cases. The shared one may be\n> used immediately, and the user can keep using it for a while without\n> \"touching\".\n\nNow you are talking about a case where the shared index file can be\nused immediately and the user can keep using it.\nThis is a different case than the case I was talking about (which is\nthe case when the shared index file does not exist anymore).\n\n> Perhaps the shared one was chowned to \"root\" while the\n> user is looking the other way, and its timestamp is now frozen to\n> the time of chown. It is a ticking time-bomb that will go off when\n> your expiry logic kicks in.\n\nIn the case I was talking about, the shared index file doesn't exist\nanymore. So there is no time ticking bomb, because what could happen,\nif we cannot freshen the shared index file, is that the shared index\nfile could be removed, which wouldn't make things worse as anyway it\ndoes not exist anymore.\n\nSo the case when the shared index file doesn't exist is different than\nother cases.\n\nNow if you want to talk about the case when the shared index file has\nnot been removed, but just chowned to \"root\", then I wonder how is it\ndifferent from the case when the file system is read only.\nPerhaps the admin just chowned everything to make sure that the repo\ncould not be changed anymore by the usual processes and users.\n\n>> So I am not sure it's a good idea to anticipate a catastrophic failure\n>> that may not happen. Perhaps we could just warn, but I am not sure it\n>> will help the user. If the catastrophic failure doesn't happen because\n>> the shared index file is not needed, I can't see how the warning could\n>> help. And if there is a catastrophic failure, the following will be\n>> displayed:\n>>\n>> fatal: .git/sharedindex.cbfe41352a4623d4d3e9757861fed53f3094e0ac:\n>> index file open failed: No such file or directory\n>\n> That \"fatal\" is given _ONLY_ after time passes and our failure to\n> \"touch\" the file that is still-in-use left it subject to \"gc\". Of\n> course, when \"fatal\" is given, it is too late to warn about ticking\n> time bomb.\n>\n> At the time we notice a ticking time bomb is the only sensible time\n> to warn. Or better yet take a corrective action.\n\nIn a previous email you wrote that if the file system is read only, it\nis a bad thing if we warn.\nSo I wonder why it would be a good thing to warn or try to do\nsomething if the admin chowned everything to prevent usual processes\nand users to change anything.\n\n> As I expect (and you seem to agree) that a failure to \"touch\" would\n> be a very rare event (like, sysadmin chowning it to \"root\" by\n> mistake),\n\nYeah, I agree that a failure to \"touch\" would be a rare event. But the\nthing is we often don't know if admins or users do things by mistake\nor not.\nIf they rm -rf everything forcefully, for example, they might not like\nit, if we start to rewrite stuff to try to \"correct\" things.\n\n> I do not mind if the \"corrective action\" were \"immediately\n> unsplit the index, so that at least the current and the latest index\n> contents will be written out safely to a new single unsplit index\n> file\".\n\nIf the admins or users chowned a shared index file precisely because\nthey want to save it from ever being garbage collected, I am not sure\nthat they would be happy if we decide to \"correct\" things.\nFor example they might not like it if we change the current mode to\nunsplit index.\n\nSo in my opinion, the safest thing, if we really want to do something,\nis to just warn, perhaps once, and I am ok to do that.\nIf we really wanted to do our best, perhaps we could warn once only in\nthe case when the shared index file still exists (because otherwise\nthere nothing to lose anymore).\n\n> That won't help _other_ split index files that are based on\n> the same \"untouchable\" shared index, but I do not think that is a\n> problem we need to solve---if they are still in use, the code that\n> use them will notice it, and otherwise they are not in use and can\n> be aged away safely.\n\nI am not sure that I understand you correctly on this. The current\ncode does not age away the split index files. It only ages away shared\nindex files.\nI think aging away the split index file is a different issue, because\nwhen we are not in split index mode, the index files are not aged\nanyway.\n"},{"id":"310612","messageId":"xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"CAP8UFD3iWg5g-h209VDyg1U03Jmma8nTONyCYB-kN7GwspkL8Q@mail.gmail.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-31T19:22:12Z","receivedAt":"2017-01-31T19:22:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> You are listing only the irrelevant cases. The shared one may be\n>> used immediately, and the user can keep using it for a while without\n>> \"touching\".\n>\n> Now you are talking about a case where the shared index file can be\n> used immediately and the user can keep using it.\n> This is a different case than the case I was talking about (which is\n> the case when the shared index file does not exist anymore).\n\nYes, as I said, you are listing irrelevant and uninteresting one.\nIf the shared index is already gone, reporting failure to touch it\nis of no use---an attempt to read and use the split index that\ndepends on the shared index that is gone will fail anyway.\n\n> In a previous email you wrote that if the file system is read only, it\n> is a bad thing if we warn.\n\nYes, but you need to realize that \"it is better not to bother users\nwith a report of failure to touch in read-only repository\" and \"we\nignore all failures\".\n\nIIUC, you attempt to touch the shared index even when you are\nonly reading the index, because you want to mark the fact that the\nshared index is still being depended upon. And I tend to agree that\nit is OK not to report a failure for that case. It is very similar\nto a situation where you are asked to peek into your coworker's\nrepository, which you do not have write access to, and run \"status\".\nThe command first runs the equivalent of \"update-index --refresh\"\nonly in-core, and it attempts to write the updated index because\n(1) it paid the cost of doing the refreshing already, and (2) if it\ncan write into the index, it will help future operations in the\nrepository. But it does not report a failure to write the index\nexactly because it is merely doing an opportunistic write.\n\nAnd in the \"we read from the split index, and we attempt to touch\nthe underlying shared one to update its timestamp\" case, it is OK\nnot to report if we failed to touch.\n\nBut you also attempt to touch the shared index when you are actually\nwriting out a new split index file based on it, no? The \"you\ncreated a ticking bomb\" situation is where you fail to touch the\nshared index for whatever reason, even when you managed to write the\nnew split index file.\n\nWe agreed that read-only operation should not nag, so it won't\ntrigger when you are peeking somebody else's repository to help him.\nAs I said, it is an uninteresting and irrelevant case---when your\nread-only peeking did not add new information to be preserved, it is\nless severe problem that you fail to reset the expiration. On the\nother hand, if you added new information, i.e. wrote the split index\nbased on it, it is a good indication that the index\npair has information that is more valuable. We must warn in that\ncase.\n\n> Now if you want to talk about the case when the shared index file has\n> not been removed, but just chowned to \"root\", then I wonder how is it\n> different from the case when the file system is read only.\n\nThe difference is that your code has enough information to notice\nthe case where you know your touch failed, you know that you wanted\nto write (and the write succeeded), and yet you do NOT know why your\ntouch failed. That is the ticking bomb we need to warn the user\nabout.\n\n"},{"id":"310621","messageId":"xmqqbmunq6mg.fsf@gitster.mtv.corp.google.com","threadId":"44768","inReplyTo":"xmqqy3xrqbkr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 14/21] read-cache: touch shared index files when used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-31T21:09:11Z","receivedAt":"2017-01-31T21:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Yes, but you need to realize that \"it is better not to bother users\n> with a report of failure to touch in read-only repository\" and \"we\n> ignore all failures\".\n\nSorry about an unfinished sentence here. \"need to realize that\n... and ... are different things.\"\n\n> ... It is very similar\n> to a situation where you ... run \"status\".\n> The command first runs the equivalent of \"update-index --refresh\"\n> only in-core, and it attempts to write the updated index because\n> (1) it paid the cost of doing the refreshing already, and (2) if it\n> can write into the index, it will help future operations in the\n> repository. But it does not report a failure to write the index\n> exactly because it is merely doing an opportunistic write.\n>\n> And in the \"we read from the split index, and we attempt to touch\n> the underlying shared one to update its timestamp\" case, it is OK\n> not to report if we failed to touch.\n> ...\n> ... On the\n> other hand, if you added new information, i.e. wrote the split index\n> based on it, it is a good indication that the index\n> pair has information that is more valuable. We must warn in that\n> case.\n\nThis reminds us of a third case. What should happen if we are doing\nthe \"opportunistic writing of the index\" in \"git status\", managed to\nwrite the split index, but failed to touch the shared one?\n\nIn the ideal world, I think we should do the following sequence:\n\n - \"status\" tries to write cache to the file.\n\n - we try to write and on any error, we return error to the caller,\n who is already prepared to ignore it and stay silent.\n\n - as the first step of writing the index, we first try to touch\n the shared one. If it fails, we return an error here without\n writing the split one out.\n\n - then we try to write the split one out. If this fails, we\n also return an error.\n\n - otherwise, both touching of the shared one and writing of the\n split one are successful. \n\n - \"status\" finishes the opportunistic refreshing of the index, by\n either ignoring an error silently (if either touching of shared\n one or writing of split one fails) or writing the refreshed index\n out successfully.\n\nIt is OK to swap the order of touching the shared one and writing of\nthe split one in the above sequence, as long as an error in either\nstep signals a failure to the opportunistic caller.\n\nI do not offhand know if the split-index code is structured in such\na way to allow the above sequence easily, or it needs refactoring.\n\nIf such a restructuring is required, it might not be within the\nscope of the series and I am OK if you just left the NEEDSWORK\ncomment that describes the above (i.e. what we should be doing) as\nan in-code comment so that we can pick it up later. The whole point\nof the step 14/21 on the other hand is to make sure that a shared\nindex that is still in active use will not go stale, and from that\npoint of view, such a \"punting\" may not be a good idea---it\ndeliberately finishes the series knowing that it does not adequately\ndo what it promises to do. \n\nSo, ... I dunno.\n\n"}]}