{"thread":{"id":"42821","subject":"[RFC/PATCH 0/8] Add configuration options for split-index","startedAt":"2016-07-11T17:23:24Z","lastAt":"2016-07-25T21:18:27Z","messageCount":19,"participants":["Christian Couder","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"291177","messageId":"20160711172254.13439-1-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":null,"subject":"[RFC/PATCH 0/8] Add configuration options for split-index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:46Z","receivedAt":"2016-07-11T17:23:24Z","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 current work to libify `git\napply`.\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\nHighlevel view of the patches in the series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n - Patches 1/8 to 4/8 introduce the functions that are reading the\n configuration variable and tweaking the split index depending on\n its value.\n\n - Patches 5/8 and 6/8 add some documentation for the new feature.\n\n - Patch 7/8 adds a few simple tests.\n\n - Patch 8/8 cleans up the old \"sharedindex.XXXX\" previously\n created by split-index.\n\nFuture work\n~~~~~~~~~~~\n\nOne thing that is probably missing is a mechanism to avoid having too\nmany changes accumulating in the (split) index while in split index\nmode. The git-update-index 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\nbut it is probably better to not expect the user to think about it and\nto have a mechanism that pushes back all changes to the shared index\nfile automatically when some threshold is reached. The threshold could\nbe for example when $GIT_DIR/index size is larger than 25% of the\nshared index size. Opinions, test results or test ideas are welcome on\nthis.\n\nLinks\n~~~~~\n\nThe last iteration of the git apply libification patch series is\nthere:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/298344/\n\nThis patch series is also available here:\n\nhttps://github.com/chriscool/git/commits/config-split-index\n\n\nChristian Couder (8):\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 Documentation/config: add information for core.splitIndex\n Documentation/git-update-index: talk about core.splitIndex config var\n t1700: add tests for core.splitIndex\n read-cache: unlink old sharedindex files\n\n Documentation/config.txt | 4 ++++\n Documentation/git-update-index.txt | 6 ++++++\n builtin/update-index.c | 20 ++++++++---------\n cache.h | 1 +\n config.c | 10 +++++++++\n read-cache.c | 44 +++++++++++++++++++++++++++++++++++++-\n split-index.c | 18 ++++++++++++++++\n split-index.h | 2 ++\n t/t1700-split-index.sh | 44 ++++++++++++++++++++++++++++++++++++++\n 9 files changed, 138 insertions(+), 11 deletions(-)\n\n-- \n2.9.0.250.g7087ccc.dirty\n\n"},{"id":"291178","messageId":"20160711172254.13439-3-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 2/8] split-index: add {add,remove}_split_index() functions","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:48Z","receivedAt":"2016-07-11T17:23:29Z","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\nSigned-off-by: Christian Couder \n---\n builtin/update-index.c | 17 ++++-------------\n split-index.c | 18 ++++++++++++++++++\n split-index.h | 2 ++\n 3 files changed, 24 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 6cdfd5f..f06fe80 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1108,19 +1108,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&buf);\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+\tif (split_index > 0)\n+\t\tadd_split_index(&the_index);\n+\telse if (!split_index && the_index.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 3c75d4b..9466b69 100644\n--- a/split-index.c\n+++ b/split-index.c\n@@ -319,3 +319,21 @@ 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+\tinit_split_index(istate);\n+\tistate->cache_changed |= SPLIT_INDEX_ORDERED;\n+}\n+\n+void remove_split_index(struct index_state *istate)\n+{\n+\t/*\n+\t * can't discard_split_index(&the_index); because that\n+\t * will destroy split_index->base->cache[], which may\n+\t * be shared with the_index.cache[]. So yeah we're\n+\t * leaking a bit here.\n+\t */\n+\tistate->split_index = NULL;\n+\tistate->cache_changed |= SOMETHING_CHANGED;\n+}\ndiff --git a/split-index.h b/split-index.h\nindex c1324f5..df91c1b 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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291179","messageId":"20160711172254.13439-4-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 3/8] read-cache: add and then use tweak_split_index()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:49Z","receivedAt":"2016-07-11T17:23:33Z","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 db27766..ae292d6 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1538,10 +1538,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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291180","messageId":"20160711172254.13439-6-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 5/8] Documentation/config: add information for core.splitIndex","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:51Z","receivedAt":"2016-07-11T17: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/config.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e208af1..aec8ecb 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -324,6 +324,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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291181","messageId":"20160711172254.13439-7-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 6/8] Documentation/git-update-index: talk about core.splitIndex config var","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:52Z","receivedAt":"2016-07-11T17:23:39Z","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 c6cbed1..2293140 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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291182","messageId":"20160711172254.13439-8-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 7/8] t1700: add tests for core.splitIndex","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:53Z","receivedAt":"2016-07-11T17:23:42Z","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 8aef49f..f1af0d5 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 <\n---\n builtin/update-index.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex f06fe80..2b8aaa6 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -1108,10 +1108,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_release(&buf);\n \t}\n \n-\tif (split_index > 0)\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\tadd_split_index(&the_index);\n-\telse if (!split_index && the_index.split_index)\n+\t} else if (!split_index && the_index.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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291184","messageId":"20160711172254.13439-9-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:54Z","receivedAt":"2016-07-11T17:23:44Z","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 makes sure that old sharedindex\nfiles are removed after a new one has been created.\n\nSigned-off-by: Christian Couder \n---\n read-cache.c | 27 ++++++++++++++++++++++++++-\n t/t1700-split-index.sh | 7 +++++++\n 2 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex ae292d6..13aa058 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2166,6 +2166,28 @@ static int write_split_index(struct index_state *istate,\n \treturn ret;\n }\n \n+static void 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\terror_errno(\"unable to open git dir: %s\", get_git_dir());\n+\t\treturn;\n+\t}\n+\n+\twhile ((de = readdir(dir)) != NULL) {\n+\t\tconst char *sha1_hex;\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\tif (unlink(git_path(\"%s\", de->d_name)))\n+\t\t\terror_errno(\"unable to unlink: %s\", git_path(\"%s\", de->d_name));\n+\t}\n+\tclosedir(dir);\n+}\n+\n static struct tempfile temporary_sharedindex;\n \n static int write_shared_index(struct index_state *istate,\n@@ -2187,8 +2209,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 \ndiff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh\nindex f1af0d5..5036efa 100755\n--- a/t/t1700-split-index.sh\n+++ b/t/t1700-split-index.sh\n@@ -237,4 +237,11 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'only one \"sharedindex\" files' '\n+\tgit config core.splitIndex true &&\n+\t: >three &&\n+\tgit update-index --add three &&\n+\ttest $(ls .git/sharedindex.* | wc -l) = 1\n+'\n+\n test_done\n-- \n2.9.0.250.g7087ccc.dirty\n\n"},{"id":"291185","messageId":"20160711172254.13439-2-chriscool@tuxfamily.org","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"[RFC/PATCH 1/8] config: add git_config_get_split_index()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-11T17:22:47Z","receivedAt":"2016-07-11T17:23:47Z","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 cache.h | 1 +\n config.c | 10 ++++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex f1dc289..5296a50 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1695,6 +1695,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 bea937e..318194b 100644\n--- a/config.c\n+++ b/config.c\n@@ -1656,6 +1656,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 = -1;\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.9.0.250.g7087ccc.dirty\n\n"},{"id":"291189","messageId":"CACsJy8BqMFASHf5kJgUh+bd7XG98CafNydE964VJyPXz-emEvA@mail.gmail.com","threadId":"42821","inReplyTo":"20160711172254.13439-9-chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-11T18:27:58Z","receivedAt":"2016-07-11T18:28:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder\n wrote:\n> Everytime split index is turned on, it creates a \"sharedindex.XXXX\"\n> file in the git directory. This makes sure that old sharedindex\n> files are removed after a new one has been created.\n\nHmm it's one-way link, we don't know how many index files use this\nshared index file, how can you be sure nobody else will need it? I'm\nthinking about temporary indexes. If a temp index is created, saved on\ndisk, and use delete the shared index file, the real, main index may\nbecome useless. Temp index will most likely replace the main index\n(git commit) but if a failure happens, we can't fall back.\n\nA safer approach is \"touch\" the shared index every time a linked index\nis used, then we can delete shared indexes with old mtime, older than\na grace period, in git-prune (or here).\n-- \nDuy\n"},{"id":"291240","messageId":"CAP8UFD01MiMnz6qNGYa9WEjZ_EOy-hBqO4gGKGyxUc71aOyp4w@mail.gmail.com","threadId":"42821","inReplyTo":"CACsJy8BqMFASHf5kJgUh+bd7XG98CafNydE964VJyPXz-emEvA@mail.gmail.com","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-12T07:04:52Z","receivedAt":"2016-07-12T07:05:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jul 11, 2016 at 8:27 PM, Duy Nguyen wrote:\n> On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder\n> wrote:\n>> Everytime split index is turned on, it creates a \"sharedindex.XXXX\"\n>> file in the git directory. This makes sure that old sharedindex\n>> files are removed after a new one has been created.\n>\n> Hmm it's one-way link, we don't know how many index files use this\n> shared index file, how can you be sure nobody else will need it? I'm\n> thinking about temporary indexes. If a temp index is created, saved on\n> disk, and use delete the shared index file, the real, main index may\n> become useless. Temp index will most likely replace the main index\n> (git commit) but if a failure happens, we can't fall back.\n\nIsn't there a way to scan all the current indexes (temp or not) to see\nwhich shared indexes they need?\n\n> A safer approach is \"touch\" the shared index every time a linked index\n> is used, then we can delete shared indexes with old mtime, older than\n> a grace period, in git-prune (or here).\n\nMaybe old linked indexes could be converted after some time to use a\nnewer shared index, so that we can get rid of the old shared indexes.\nThat seems safer than just deleting old linked indexes.\n\nThanks for your review,\nChristian.\n"},{"id":"291263","messageId":"CACsJy8CEimmc=W=bNcLw+TP0tGRPCL5rr3uNeFMJ=Ms9HkyABw@mail.gmail.com","threadId":"42821","inReplyTo":"CAP8UFD01MiMnz6qNGYa9WEjZ_EOy-hBqO4gGKGyxUc71aOyp4w@mail.gmail.com","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-12T15:12:24Z","receivedAt":"2016-07-12T15:12:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 12, 2016 at 9:04 AM, Christian Couder\n wrote:\n> On Mon, Jul 11, 2016 at 8:27 PM, Duy Nguyen wrote:\n>> On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder\n>> wrote:\n>>> Everytime split index is turned on, it creates a \"sharedindex.XXXX\"\n>>> file in the git directory. This makes sure that old sharedindex\n>>> files are removed after a new one has been created.\n>>\n>> Hmm it's one-way link, we don't know how many index files use this\n>> shared index file, how can you be sure nobody else will need it? I'm\n>> thinking about temporary indexes. If a temp index is created, saved on\n>> disk, and use delete the shared index file, the real, main index may\n>> become useless. Temp index will most likely replace the main index\n>> (git commit) but if a failure happens, we can't fall back.\n>\n> Isn't there a way to scan all the current indexes (temp or not) to see\n> which shared indexes they need?\n\nNo. People could create an index file anywhere in theory. So you don't\nknow how many index files there are.\n\n>> A safer approach is \"touch\" the shared index every time a linked index\n>> is used, then we can delete shared indexes with old mtime, older than\n>> a grace period, in git-prune (or here).\n>\n> Maybe old linked indexes could be converted after some time to use a\n> newer shared index, so that we can get rid of the old shared indexes.\n> That seems safer than just deleting old linked indexes.\n\nIt really depends. If the shared part is too small for old indexes, we\nmight as well unsplit them. In practice though, the only long-term\nindex file is $GIT_DIR/index. If we don't delete old shared index\nfiles too close to their creation time, temp index files will go away.\n-- \nDuy\n"},{"id":"291273","messageId":"CACsJy8ApNJ+EBOVNC0GoPUaO898QRqHUZ2yQ39Qg5uGq4sHHGg@mail.gmail.com","threadId":"42821","inReplyTo":"20160711172254.13439-1-chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 0/8] Add configuration options for split-index","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-12T16:01:12Z","receivedAt":"2016-07-12T16:01:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder\n wrote:\n> Future work\n> ~~~~~~~~~~~\n>\n> One thing that is probably missing is a mechanism to avoid having too\n> many changes accumulating in the (split) index while in split index\n> mode. The git-update-index documentation says:\n>\n> If 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.\n>\n> but it is probably better to not expect the user to think about it and\n> to have a mechanism that pushes back all changes to the shared index\n> file automatically when some threshold is reached. The threshold could\n> be for example when $GIT_DIR/index size is larger than 25% of the\n> shared index size. Opinions, test results or test ideas are welcome on\n> this.\n\nOh yes I would like something like this. I stuck to the basics because\nas you see you need to define some criteria to re-split again, but\nwithout experimenting on real repos, I could just have gone the wrong\nway. Index file size or the percentage of entries in linked/shared\nindexes are two good candidates. You can also just re-split on\ncommands that likely lead to increasing linked index size a lot (maybe\ngit-reset), or run long enough that some extra processing won't get\nnoticed. For example git-gc should definitely re-split if this feature\nis on, but I can't say if it's often enough.\n-- \nDuy\n"},{"id":"291299","messageId":"CAP8UFD2pes3MD9FNBtvLYS5785aJoGPz00Fj+fHjy03j0aKjFg@mail.gmail.com","threadId":"42821","inReplyTo":"CACsJy8CEimmc=W=bNcLw+TP0tGRPCL5rr3uNeFMJ=Ms9HkyABw@mail.gmail.com","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-12T19:45:25Z","receivedAt":"2016-07-12T19:45:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jul 12, 2016 at 5:12 PM, Duy Nguyen wrote:\n>\n> No. People could create an index file anywhere in theory. So you don't\n> know how many index files there are.\n\nMaybe when an index file is created, its path and its sharedindex file\ncould be appended into a log file.\nWe could check this log file to see if we can remove sharedindex files.\nWe would need to remove the entries in this log file for the indexes\nthat are no longer there.\n\nOr instead of one log file we could have a file for each index file in\na special directory called for example \"indexinfo\".\nSo we could just delete the file if its related index is no longer there.\n\n> It really depends. If the shared part is too small for old indexes, we\n> might as well unsplit them. In practice though, the only long-term\n> index file is $GIT_DIR/index. If we don't delete old shared index\n> files too close to their creation time, temp index files will go away.\n\nWe could treat $GIT_DIR/index specially so that if there are no temp\nindex files, there should be nothing in \"indexinfo\".\n"},{"id":"291345","messageId":"CACsJy8DHd44MnFUkJwh9n0JK5J5P6kER0kgopX0UrS=ROo_URQ@mail.gmail.com","threadId":"42821","inReplyTo":"CAP8UFD2pes3MD9FNBtvLYS5785aJoGPz00Fj+fHjy03j0aKjFg@mail.gmail.com","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-13T15:16:25Z","receivedAt":"2016-07-13T15:21:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jul 12, 2016 at 9:45 PM, Christian Couder\n wrote:\n> On Tue, Jul 12, 2016 at 5:12 PM, Duy Nguyen wrote:\n>>\n>> No. People could create an index file anywhere in theory. So you don't\n>> know how many index files there are.\n>\n> Maybe when an index file is created, its path and its sharedindex file\n> could be appended into a log file.\n> We could check this log file to see if we can remove sharedindex files.\n> We would need to remove the entries in this log file for the indexes\n> that are no longer there.\n>\n> Or instead of one log file we could have a file for each index file in\n> a special directory called for example \"indexinfo\".\n> So we could just delete the file if its related index is no longer there.\n\nNew files will require locking so people don't append at the same\ntime. And maybe another new host of problems. I think we can just go\nwith the garbage collection way that we have done for unreachable\nobjects.\n\nYour indexinfo idea looks very close to multiple locking, an index\nwould lock the shared index it's linked to, preventing it from being\nremoved. For single locking, we can just create a file named $X.lock,\nbut for multiple locks, maybe we can go with\n$X.lock-$index_trailing_sha1. Will it work? I don't know. Just\nthinking out loud.\n\n>> It really depends. If the shared part is too small for old indexes, we\n>> might as well unsplit them. In practice though, the only long-term\n>> index file is $GIT_DIR/index. If we don't delete old shared index\n>> files too close to their creation time, temp index files will go away.\n>\n> We could treat $GIT_DIR/index specially so that if there are no temp\n> index files, there should be nothing in \"indexinfo\".\n\nNo, temp index files are needed. And it will be hard to treat\n$GIT_DIR/index specially because updating it involves a temp index:\nyou first prepare a new index in $GIT_DIR/index.lock. If everything\ngoes well, you atomically rename it to $GIT_DIR/index. You may be able\nto treat $GIT_DIR/index.lock special too, but that's starting to get\nout of hand.\n-- \nDuy\n"},{"id":"291372","messageId":"CAP8UFD1=TLG=04BhjGWRa=M+8=4QGcrH3aj4VB8kb5pjXPHm8w@mail.gmail.com","threadId":"42821","inReplyTo":"CACsJy8DHd44MnFUkJwh9n0JK5J5P6kER0kgopX0UrS=ROo_URQ@mail.gmail.com","subject":"Re: [RFC/PATCH 8/8] read-cache: unlink old sharedindex files","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-13T17:54:28Z","receivedAt":"2016-07-13T17:56:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jul 13, 2016 at 5:16 PM, Duy Nguyen wrote:\n> On Tue, Jul 12, 2016 at 9:45 PM, Christian Couder\n> wrote:\n>> On Tue, Jul 12, 2016 at 5:12 PM, Duy Nguyen wrote:\n>>>\n>>> No. People could create an index file anywhere in theory. So you don't\n>>> know how many index files there are.\n>>\n>> Maybe when an index file is created, its path and its sharedindex file\n>> could be appended into a log file.\n>> We could check this log file to see if we can remove sharedindex files.\n>> We would need to remove the entries in this log file for the indexes\n>> that are no longer there.\n>>\n>> Or instead of one log file we could have a file for each index file in\n>> a special directory called for example \"indexinfo\".\n>> So we could just delete the file if its related index is no longer there.\n>\n> New files will require locking so people don't append at the same\n> time.\n\nIf we create a new file for each index file with a name that depends\non the index path (for example maybe the sha1 of the index path) then\nas many index files with the same path are not possible we should be\nsafe.\n\n> And maybe another new host of problems. I think we can just go\n> with the garbage collection way that we have done for unreachable\n> objects.\n>\n> Your indexinfo idea looks very close to multiple locking, an index\n> would lock the shared index it's linked to, preventing it from being\n> removed. For single locking, we can just create a file named $X.lock,\n> but for multiple locks, maybe we can go with\n> $X.lock-$index_trailing_sha1. Will it work? I don't know. Just\n> thinking out loud.\n\nIsn't is possible that the same index (so with the same trailing sha1)\nbe created in two different places?\n\n>>> It really depends. If the shared part is too small for old indexes, we\n>>> might as well unsplit them. In practice though, the only long-term\n>>> index file is $GIT_DIR/index. If we don't delete old shared index\n>>> files too close to their creation time, temp index files will go away.\n>>\n>> We could treat $GIT_DIR/index specially so that if there are no temp\n>> index files, there should be nothing in \"indexinfo\".\n>\n> No, temp index files are needed. And it will be hard to treat\n> $GIT_DIR/index specially because updating it involves a temp index:\n> you first prepare a new index in $GIT_DIR/index.lock.\n\nIf the new index is always prepared in $GIT_DIR/index.lock, then there\nis no problem in the first place because when my original patch calls\nclean_shared_index_files() the $GIT_DIR/index.lock is taken. Or maybe\nI am missing something?\n\n> If everything\n> goes well, you atomically rename it to $GIT_DIR/index. You may be able\n> to treat $GIT_DIR/index.lock special too, but that's starting to get\n> out of hand.\n"},{"id":"292040","messageId":"CAP8UFD20WtCPLb3VzCJnYZSreUM7Q8GOC_VBG3h1j3wjOcR=_w@mail.gmail.com","threadId":"42821","inReplyTo":"CACsJy8ApNJ+EBOVNC0GoPUaO898QRqHUZ2yQ39Qg5uGq4sHHGg@mail.gmail.com","subject":"Re: [RFC/PATCH 0/8] Add configuration options for split-index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-23T16:11:59Z","receivedAt":"2016-07-23T16:12:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Jul 12, 2016 at 6:01 PM, Duy Nguyen wrote:\n> On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder\n> wrote:\n>> Future work\n>> ~~~~~~~~~~~\n>>\n>> One thing that is probably missing is a mechanism to avoid having too\n>> many changes accumulating in the (split) index while in split index\n>> mode. The git-update-index documentation says:\n>>\n>> If 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.\n>>\n>> but it is probably better to not expect the user to think about it and\n>> to have a mechanism that pushes back all changes to the shared index\n>> file automatically when some threshold is reached. The threshold could\n>> be for example when $GIT_DIR/index size is larger than 25% of the\n>> shared index size. Opinions, test results or test ideas are welcome on\n>> this.\n>\n> Oh yes I would like something like this. I stuck to the basics because\n> as you see you need to define some criteria to re-split again, but\n> without experimenting on real repos, I could just have gone the wrong\n> way. Index file size or the percentage of entries in linked/shared\n> indexes are two good candidates.\n\nOk, I started working on automatically pushing back all changes to the\nshared index when the percentage of entries in linked vs shared\nindexes is greater than 25% (maybe I will make it configurable later).\nIt is very basic and doesn't work well for now (for one thing it is\nmissing added entries), see:\n\nhttps://github.com/chriscool/git/commits/config-split-index8\n\nBasically I would like a way to count then entries that are only in\nthe linked index without modifying them to be safe, but I have a hard\ntime seeing how I could modify prepare_to_write_split_index() to get\nthat.\n\n> You can also just re-split on\n> commands that likely lead to increasing linked index size a lot (maybe\n> git-reset), or run long enough that some extra processing won't get\n> noticed. For example git-gc should definitely re-split if this feature\n> is on, but I can't say if it's often enough.\n\nI'd like to avoid re-split only on some specific commands as I feel it\ncould lead to bad behavior when none of these specific commands are\ncalled but the linked index is growing because of other commands.\n"},{"id":"292098","messageId":"CACsJy8DRmcUzG+Uj65nqmyjbohbWq1Us4c7YkNkG15fjGijfEA@mail.gmail.com","threadId":"42821","inReplyTo":"CAP8UFD20WtCPLb3VzCJnYZSreUM7Q8GOC_VBG3h1j3wjOcR=_w@mail.gmail.com","subject":"Re: [RFC/PATCH 0/8] Add configuration options for split-index","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-25T16:04:56Z","receivedAt":"2016-07-25T16:05:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jul 23, 2016 at 6:11 PM, Christian Couder\n wrote:\n> Ok, I started working on automatically pushing back all changes to the\n> shared index when the percentage of entries in linked vs shared\n> indexes is greater than 25% (maybe I will make it configurable later).\n> It is very basic and doesn't work well for now (for one thing it is\n> missing added entries), see:\n>\n> https://github.com/chriscool/git/commits/config-split-index8\n>\n> Basically I would like a way to count then entries that are only in\n> the linked index without modifying them to be safe, but I have a hard\n> time seeing how I could modify prepare_to_write_split_index() to get\n> that.\n\nHmm.. can you do the counting separately? A shared cache_entry must\nhave its field \"index\" greater than zero. By counting the number of\nentries whose index is zero (i.e. not shared) against the total number\nof real (*) entries, you should have a decent estimate when to split.\nThen you can do exactly what \"git update-index --no-split-index\" and\n\"git update-index --split-index\" sequence does, but in write_index().\nIt's easier than messing inside split-index.c. If we hit performance\nproblem, then we can look into changing split-index.c\n\n(*) remember that some entries may be marked CE_REMOVE, which are dead\nentries and should not be counted because they will never be written\ndown on disk.\n-- \nDuy\n"},{"id":"292143","messageId":"CAP8UFD16VearVw3EMM_TzV=43qvyoo2PBDhEOFJFuBU5=Pe4Lw@mail.gmail.com","threadId":"42821","inReplyTo":"CACsJy8DRmcUzG+Uj65nqmyjbohbWq1Us4c7YkNkG15fjGijfEA@mail.gmail.com","subject":"Re: [RFC/PATCH 0/8] Add configuration options for split-index","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-07-25T21:18:04Z","receivedAt":"2016-07-25T21:18:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Jul 25, 2016 at 6:04 PM, Duy Nguyen wrote:\n>\n> Hmm.. can you do the counting separately? A shared cache_entry must\n> have its field \"index\" greater than zero. By counting the number of\n> entries whose index is zero (i.e. not shared) against the total number\n> of real (*) entries, you should have a decent estimate when to split.\n> Then you can do exactly what \"git update-index --no-split-index\" and\n> \"git update-index --split-index\" sequence does, but in write_index().\n> It's easier than messing inside split-index.c. If we hit performance\n> problem, then we can look into changing split-index.c\n\nYeah, thanks for the suggestion. I will try it.\n\n> (*) remember that some entries may be marked CE_REMOVE, which are dead\n> entries and should not be counted because they will never be written\n> down on disk.\n"}]}