{"thread":{"id":"36744","subject":"[BUG] auto-repack exits prematurely, locking other processing out","startedAt":"2014-05-23T19:51:21Z","lastAt":"2014-05-27T18:09:03Z","messageCount":8,"participants":["Adam Borowski","Junio C Hamano","Duy Nguyen","Nguyễn Thái Ngọc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"242605","messageId":"20140523195121.GA923@angband.pl","threadId":"36744","inReplyTo":null,"subject":"[BUG] auto-repack exits prematurely, locking other processing out","fromName":"Adam Borowski","fromEmail":"kilobyte@angband.pl","sentAt":"2014-05-23T19:51:21Z","receivedAt":"2014-05-23T19:51:21Z","isPatch":false,"sender":{"key":"kilobyte@angband.pl","avatar":"https://avatars.githubusercontent.com/u/48801?v=4"},"body":"Hi guys!\n\nIt looks like the periodic auto-repack backgrounds itself when it shouldn't\ndo so. This causes the command it has triggered as a part of to fail:\n\n==========================================================================\n[~/linux](master)$ git pull --rebase\nremote: Counting objects: 455, done.\nremote: Compressing objects: 100% (64/64), done.\nremote: Total 267 (delta 208), reused 262 (delta 203)\nReceiving objects: 100% (267/267), 44.43 KiB | 0 bytes/s, done.\nResolving deltas: 100% (208/208), completed with 80 local objects.\nFrom git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux\n 4b660a7..f02f79d master -> linus/master\nAuto packing the repository in background for optimum performance.\nSee \"git help gc\" for manual housekeeping.\nFirst, rewinding head to replay your work on top of it...\nApplying: perf: tools: fix missing casts for printf arguments.\nApplying: vt: emulate 8- and 24-bit colour codes.\nfatal: Unable to create '/home/kilobyte/linux/.git/refs/heads/master.lock': File exists.\n\nIf no other git process is currently running, this probably means a\ngit process crashed in this repository earlier. Make sure no other git\nprocess is running and remove the file manually to continue.\nCould not move back to refs/heads/master\n[~/linux]((no branch, rebasing (null)))$\n==========================================================================\n\n-- \nGnome 3, Windows 8, Slashdot Beta, now Firefox Ribbon^WAustralis. WTF is going\non with replacing usable interfaces with tabletized ones?\n"},{"id":"242616","messageId":"xmqqy4xsgome.fsf@gitster.dls.corp.google.com","threadId":"36744","inReplyTo":"20140523195121.GA923@angband.pl","subject":"Re: [BUG] auto-repack exits prematurely, locking other processing out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-23T21:40:41Z","receivedAt":"2014-05-23T21:40:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Borowski writes:\n\n> Hi guys!\n>\n> It looks like the periodic auto-repack backgrounds itself when it shouldn't\n> do so. This causes the command it has triggered as a part of to fail:\n\nYikes. In the meantime, I think you can turn gc.autodetach off as a\nworkaround, e.g.\n\n $ git config --global --add gc.autodetach off\n\nDuy, 9f673f94 (gc: config option for running --auto in background,\n2014-02-08) turns to be not such a hot idea. Sure, if we kick it\noff background after doing something heavy, immediately before\ngiving control back to the end-user, and expect that the user will\nstay thinking without making new changes (i.e. read-only stuff like\n\"git show\" would be OK), then daemonize might be a great thing, but\nwe forgot, while doing that commit, that long-running operations\ntrigger the auto gc in the middle *and* they want it finish before\nthey continue, as the purpose of gc is to help the performance\nduring their further operation.\n\n\n\n>\n> ==========================================================================\n> [~/linux](master)$ git pull --rebase\n> remote: Counting objects: 455, done.\n> remote: Compressing objects: 100% (64/64), done.\n> remote: Total 267 (delta 208), reused 262 (delta 203)\n> Receiving objects: 100% (267/267), 44.43 KiB | 0 bytes/s, done.\n> Resolving deltas: 100% (208/208), completed with 80 local objects.\n> From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux\n> 4b660a7..f02f79d master -> linus/master\n> Auto packing the repository in background for optimum performance.\n> See \"git help gc\" for manual housekeeping.\n> First, rewinding head to replay your work on top of it...\n> Applying: perf: tools: fix missing casts for printf arguments.\n> Applying: vt: emulate 8- and 24-bit colour codes.\n> fatal: Unable to create '/home/kilobyte/linux/.git/refs/heads/master.lock': File exists.\n>\n> If no other git process is currently running, this probably means a\n> git process crashed in this repository earlier. Make sure no other git\n> process is running and remove the file manually to continue.\n> Could not move back to refs/heads/master\n> [~/linux]((no branch, rebasing (null)))$\n> ==========================================================================\n"},{"id":"242622","messageId":"20140523223437.GA4230@angband.pl","threadId":"36744","inReplyTo":"xmqqy4xsgome.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG] auto-repack exits prematurely, locking other processing out","fromName":"Adam Borowski","fromEmail":"kilobyte@angband.pl","sentAt":"2014-05-23T22:34:37Z","receivedAt":"2014-05-23T22:34:37Z","isPatch":false,"sender":{"key":"kilobyte@angband.pl","avatar":"https://avatars.githubusercontent.com/u/48801?v=4"},"body":"On Fri, May 23, 2014 at 02:40:41PM -0700, Junio C Hamano wrote:\n> Adam Borowski writes:\n> > It looks like the periodic auto-repack backgrounds itself when it shouldn't\n> > do so. This causes the command it has triggered as a part of to fail:\n> \n> Duy, 9f673f94 (gc: config option for running --auto in background,\n> 2014-02-08) turns to be not such a hot idea. Sure, if we kick it\n> off background after doing something heavy, immediately before\n> giving control back to the end-user, and expect that the user will\n> stay thinking without making new changes (i.e. read-only stuff like\n> \"git show\" would be OK), then daemonize might be a great thing, but\n> we forgot, while doing that commit, that long-running operations\n> trigger the auto gc in the middle *and* they want it finish before\n> they continue, as the purpose of gc is to help the performance\n> during their further operation.\n\nJust add a lock that's triggered by daemonize, and have things block on this\nlock. This handles all cases:\n* --auto in the middle of a command: the block will kick in immediately,\n effectively reverting to non-daemonized version\n* --auto at the end, the user does nothing: gc will finish its work in the\n background, just as you wanted\n* --auto at the end, the user starts a new write two seconds later: gc works\n in the foreground with those 2 seconds headstart\n\nThe only loss is the lack of a progress indicator, and even that can be\ndone.\n\n-- \nGnome 3, Windows 8, Slashdot Beta, now Firefox Ribbon^WAustralis. WTF is going\non with replacing usable interfaces with tabletized ones?\n"},{"id":"242624","messageId":"xmqqlhtsglr9.fsf@gitster.dls.corp.google.com","threadId":"36744","inReplyTo":"20140523223437.GA4230@angband.pl","subject":"Re: [BUG] auto-repack exits prematurely, locking other processing out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-23T22:42:34Z","receivedAt":"2014-05-23T22:42:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Borowski writes:\n\n> On Fri, May 23, 2014 at 02:40:41PM -0700, Junio C Hamano wrote:\n>> Adam Borowski writes:\n>> > It looks like the periodic auto-repack backgrounds itself when it shouldn't\n>> > do so. This causes the command it has triggered as a part of to fail:\n>> \n>> Duy, 9f673f94 (gc: config option for running --auto in background,\n>> 2014-02-08) turns to be not such a hot idea. Sure, if we kick it\n>> off background after doing something heavy, immediately before\n>> giving control back to the end-user, and expect that the user will\n>> stay thinking without making new changes (i.e. read-only stuff like\n>> \"git show\" would be OK), then daemonize might be a great thing, but\n>> we forgot, while doing that commit, that long-running operations\n>> trigger the auto gc in the middle *and* they want it finish before\n>> they continue, as the purpose of gc is to help the performance\n>> during their further operation.\n>\n> Just add a lock that's triggered by daemonize, and have things block on this\n> lock.\n\nHmph, it defeats the whole point of running it in the background,\ndoesn't it? How would \"blocking on the lock\" be different from\nlaunching \"gc --auto\" and waiting for it to come back?\n\nAnd it would also require addition of the big-repository-lock and\ncode to take the lock sprinkled all over the place. I am not sure\nif we want to go there...\n"},{"id":"242628","messageId":"CACsJy8BfziZ7ciyKL0+X3rT9EfH_0E8nKNu9mTb_WSeTYWix_Q@mail.gmail.com","threadId":"36744","inReplyTo":"xmqqy4xsgome.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG] auto-repack exits prematurely, locking other processing out","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-24T01:26:56Z","receivedAt":"2014-05-24T01:26:56Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 24, 2014 at 4:40 AM, Junio C Hamano wrote:\n> Duy, 9f673f94 (gc: config option for running --auto in background,\n> 2014-02-08) turns to be not such a hot idea. Sure, if we kick it\n> off background after doing something heavy, immediately before\n> giving control back to the end-user, and expect that the user will\n> stay thinking without making new changes (i.e. read-only stuff like\n> \"git show\" would be OK), then daemonize might be a great thing, but\n> we forgot, while doing that commit, that long-running operations\n> trigger the auto gc in the middle *and* they want it finish before\n> they continue, as the purpose of gc is to help the performance\n> during their further operation.\n\nIf by \"long-running operations\" you mean in a single process, it's my\nfirst thought too but it looks like autogc is always called when the\nprocess is all done and about to exit. The \"git pull\" case is\ndifferent because there's rebase after fetch. I see no easy way to\ndetect this kind of \"middle of operation\".\n\nSo we have two options: scripts should disable autogc before doing\nthings, a env variable would be more convenient than temporarily\nupdating gc.auto. Or we move \"pack-refs\" and \"reflog expire\" up,\nbefore turning gc into a background task. Any locking will be\nserialized this way. We could even go further to keep all but \"repack\"\nin the background because it's \"repack\" that takes the longest time\n(maybe \"prune\" coming close to second).\n-- \nDuy\n"},{"id":"242643","messageId":"CACsJy8C3KLfh9haFh==OyGm5Hsf02i8dUVLxyLtdJEup49XhrA@mail.gmail.com","threadId":"36744","inReplyTo":"1400978309-25235-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] gc --auto: do not lock refs in the background","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-25T00:32:50Z","receivedAt":"2014-05-25T00:32:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 25, 2014 at 7:38 AM, Nguyễn Thái Ngọc Duy wrote:\n> Keep running pack-refs and \"reflog --prune\" in foreground to stop\n> parallel ref updates. The remaining background operations (repack,\n> prune and rerere) should impact running git processes.\n\nEck.. s/should impact/should not impact/\n-- \nDuy\n"},{"id":"242642","messageId":"1400978309-25235-1-git-send-email-pclouds@gmail.com","threadId":"36744","inReplyTo":"CACsJy8BfziZ7ciyKL0+X3rT9EfH_0E8nKNu9mTb_WSeTYWix_Q@mail.gmail.com","subject":"[PATCH] gc --auto: do not lock refs in the background","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-25T00:38:29Z","receivedAt":"2014-05-25T00:38:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"9f673f9 (gc: config option for running --auto in background -\n2014-02-08) puts \"gc --auto\" in background to reduce user's wait\ntime. Part of the garbage collecting is pack-refs and pruning\nreflogs. These require locking some refs and may abort other processes\ntrying to lock the same ref. If gc --auto is fired in the middle of a\nscript, gc's holding locks in the background could fail the script,\nwhich could never happen before 9f673f9.\n\nKeep running pack-refs and \"reflog --prune\" in foreground to stop\nparallel ref updates. The remaining background operations (repack,\nprune and rerere) should impact running git processes.\n\nReported-by: Adam Borowski \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/gc.c | 26 ++++++++++++++++++++------\n 1 file changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 85f5c2b..8d219d8 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -26,6 +26,7 @@ static const char * const builtin_gc_usage[] = {\n };\n \n static int pack_refs = 1;\n+static int prune_reflogs = 1;\n static int aggressive_depth = 250;\n static int aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n@@ -258,6 +259,19 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \treturn NULL;\n }\n \n+static int gc_before_repack(void)\n+{\n+\tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n+\t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n+\n+\tif (prune_reflogs && run_command_v_opt(reflog.argv, RUN_GIT_CMD))\n+\t\treturn error(FAILED_RUN, reflog.argv[0]);\n+\n+\tpack_refs = 0;\n+\tprune_reflogs = 0;\n+\treturn 0;\n+}\n+\n int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n \tint aggressive = 0;\n@@ -320,12 +334,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n \t\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n \t\t}\n-\t\tif (detach_auto)\n+\t\tif (detach_auto) {\n+\t\t\tif (gc_before_repack())\n+\t\t\t\treturn -1;\n \t\t\t/*\n \t\t\t * failure to daemonize is ok, we'll continue\n \t\t\t * in foreground\n \t\t\t */\n \t\t\tdaemonize();\n+\t\t}\n \t} else\n \t\tadd_repack_all_option();\n \n@@ -337,11 +354,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t name, (uintmax_t)pid);\n \t}\n \n-\tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n-\t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n-\n-\tif (run_command_v_opt(reflog.argv, RUN_GIT_CMD))\n-\t\treturn error(FAILED_RUN, reflog.argv[0]);\n+\tif (gc_before_repack())\n+\t\treturn -1;\n \n \tif (run_command_v_opt(repack.argv, RUN_GIT_CMD))\n \t\treturn error(FAILED_RUN, repack.argv[0]);\n-- \n1.9.1.346.ga2b5940\n"},{"id":"242756","messageId":"xmqq8upngklc.fsf@gitster.dls.corp.google.com","threadId":"36744","inReplyTo":"1400978309-25235-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] gc --auto: do not lock refs in the background","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-27T18:09:03Z","receivedAt":"2014-05-27T18:09:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> 9f673f9 (gc: config option for running --auto in background -\n> 2014-02-08) puts \"gc --auto\" in background to reduce user's wait\n> time. Part of the garbage collecting is pack-refs and pruning\n> reflogs. These require locking some refs and may abort other processes\n> trying to lock the same ref. If gc --auto is fired in the middle of a\n> script, gc's holding locks in the background could fail the script,\n> which could never happen before 9f673f9.\n>\n> Keep running pack-refs and \"reflog --prune\" in foreground to stop\n> parallel ref updates. The remaining background operations (repack,\n> prune and rerere) should impact running git processes.\n>\n> Reported-by: Adam Borowski \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/gc.c | 26 ++++++++++++++++++++------\n> 1 file changed, 20 insertions(+), 6 deletions(-)\n\nOK, as it happens the order of various gc phases we have is already\nto run pack-refs and reflog expire before everything else, so this\nchange does not affect semantics, which is good ;-)\n\n\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 85f5c2b..8d219d8 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -26,6 +26,7 @@ static const char * const builtin_gc_usage[] = {\n> };\n> \n> static int pack_refs = 1;\n> +static int prune_reflogs = 1;\n> static int aggressive_depth = 250;\n> static int aggressive_window = 250;\n> static int gc_auto_threshold = 6700;\n> @@ -258,6 +259,19 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n> \treturn NULL;\n> }\n> \n> +static int gc_before_repack(void)\n> +{\n> +\tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n> +\t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n> +\n> +\tif (prune_reflogs && run_command_v_opt(reflog.argv, RUN_GIT_CMD))\n> +\t\treturn error(FAILED_RUN, reflog.argv[0]);\n> +\n> +\tpack_refs = 0;\n> +\tprune_reflogs = 0;\n> +\treturn 0;\n> +}\n> +\n> int cmd_gc(int argc, const char **argv, const char *prefix)\n> {\n> \tint aggressive = 0;\n> @@ -320,12 +334,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n> \t\t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n> \t\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n> \t\t}\n> -\t\tif (detach_auto)\n> +\t\tif (detach_auto) {\n> +\t\t\tif (gc_before_repack())\n> +\t\t\t\treturn -1;\n> \t\t\t/*\n> \t\t\t * failure to daemonize is ok, we'll continue\n> \t\t\t * in foreground\n> \t\t\t */\n> \t\t\tdaemonize();\n> +\t\t}\n> \t} else\n> \t\tadd_repack_all_option();\n> \n> @@ -337,11 +354,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n> \t\t name, (uintmax_t)pid);\n> \t}\n> \n> -\tif (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))\n> -\t\treturn error(FAILED_RUN, pack_refs_cmd.argv[0]);\n> -\n> -\tif (run_command_v_opt(reflog.argv, RUN_GIT_CMD))\n> -\t\treturn error(FAILED_RUN, reflog.argv[0]);\n> +\tif (gc_before_repack())\n> +\t\treturn -1;\n> \n> \tif (run_command_v_opt(repack.argv, RUN_GIT_CMD))\n> \t\treturn error(FAILED_RUN, repack.argv[0]);\n"}]}