Re: [PATCH v6] gc: ignore old gc.log files

2017-02-13 Thread Junio C Hamano
David Turner  writes:

> +static unsigned long gc_log_expire_time;
> +static const char *gc_log_expire = "1.day.ago";

OK.

> @@ -113,6 +133,8 @@ static void gc_config(void)
>   git_config_get_bool("gc.autodetach", &detach_auto);
>   git_config_date_string("gc.pruneexpire", &prune_expire);
>   git_config_date_string("gc.worktreepruneexpire", 
> &prune_worktrees_expire);
> + git_config_date_string("gc.logexpiry", &gc_log_expire);
> +

OK.

I think I had a stale one queued; will replace.

Thanks.


[PATCH v6] gc: ignore old gc.log files

2017-02-10 Thread David Turner
A server can end up in a state where there are lots of unreferenced
loose objects (say, because many users are doing a bunch of rebasing
and pushing their rebased branches).  Running "git gc --auto" in
this state would cause a gc.log file to be created, preventing
future auto gcs, causing pack files to pile up.  Since many git
operations are O(n) in the number of pack files, this would lead to
poor performance.

Git should never get itself into a state where it refuses to do any
maintenance, just because at some point some piece of the maintenance
didn't make progress.

Teach Git to ignore gc.log files which are older than (by default)
one day old, which can be tweaked via the gc.logExpiry configuration
variable.  That way, these pack files will get cleaned up, if
necessary, at least once per day.  And operators who find a need for
more-frequent gcs can adjust gc.logExpiry to meet their needs.

There is also some cleanup: a successful manual gc, or a
warning-free auto gc with an old log file, will remove any old
gc.log files.

It might still happen that manual intervention is required
(e.g. because the repo is corrupt), but at the very least it won't
be because Git is too dumb to try again.

Signed-off-by: David Turner 
Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt |  6 +
 builtin/gc.c | 57 ++--
 t/t6500-gc.sh| 15 +
 3 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc5a28a32..a684b7e3e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1402,6 +1402,12 @@ gc.autoDetach::
Make `git gc --auto` return immediately and run in background
if the system supports it. Default is true.
 
+gc.logExpiry::
+   If the file gc.log exists, then `git gc --auto` won't run
+   unless that file is more than 'gc.logExpiry' old.  Default is
+   "1.day".  See `gc.pruneExpire` for more ways to specify its
+   value.
+
 gc.packRefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/builtin/gc.c b/builtin/gc.c
index 331f21926..a2b9e8924 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,6 +33,8 @@ static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
 static int detach_auto = 1;
+static unsigned long gc_log_expire_time;
+static const char *gc_log_expire = "1.day.ago";
 static const char *prune_expire = "2.weeks.ago";
 static const char *prune_worktrees_expire = "3.months.ago";
 
@@ -76,10 +78,28 @@ static void git_config_date_string(const char *key, const 
char **output)
 static void process_log_file(void)
 {
struct stat st;
-   if (!fstat(get_lock_file_fd(&log_lock), &st) && st.st_size)
+   if (fstat(get_lock_file_fd(&log_lock), &st)) {
+   /*
+* Perhaps there was an i/o error or another
+* unlikely situation.  Try to make a note of
+* this in gc.log along with any existing
+* messages.
+*/
+   int saved_errno = errno;
+   fprintf(stderr, _("Failed to fstat %s: %s"),
+   get_tempfile_path(&log_lock.tempfile),
+   strerror(saved_errno));
+   fflush(stderr);
commit_lock_file(&log_lock);
-   else
+   errno = saved_errno;
+   } else if (st.st_size) {
+   /* There was some error recorded in the lock file */
+   commit_lock_file(&log_lock);
+   } else {
+   /* No error, clean up any old gc.log */
+   unlink(git_path("gc.log"));
rollback_lock_file(&log_lock);
+   }
 }
 
 static void process_log_file_at_exit(void)
@@ -113,6 +133,8 @@ static void gc_config(void)
git_config_get_bool("gc.autodetach", &detach_auto);
git_config_date_string("gc.pruneexpire", &prune_expire);
git_config_date_string("gc.worktreepruneexpire", 
&prune_worktrees_expire);
+   git_config_date_string("gc.logexpiry", &gc_log_expire);
+
git_config(git_default_config, NULL);
 }
 
@@ -290,19 +312,34 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
 static int report_last_gc_error(void)
 {
struct strbuf sb = STRBUF_INIT;
-   int ret;
+   int ret = 0;
+   struct stat st;
+   char *gc_log_path = git_pathdup("gc.log");
 
-   ret = strbuf_read_file(&sb, git_path("gc.log"), 0);
+   if (stat(gc_log_path, &st)) {
+   if (errno == ENOENT)
+   goto done;
+
+   ret = error_errno(_("Can't stat %s"), gc_log_path);
+   goto done;
+   }
+
+   if (st.st_mtime < gc_log_expire_time)
+   goto done;
+
+   ret = strbuf_read_file(&sb, gc_log_path, 0);
if (r