threads / patch / 37464

patchgc --auto: do not run 'pack-refs' and 'reflog expire' twice

Subject: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice

## tl;dr

4 messages between Aug 31, 2014 and Sep 4, 2014. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Nguyễn Thái Ngọc Duy· Aug 31, 2014, 04:33 UTC · lore

In the --auto code path, gc_before_repack() is called once in parent process then again in the forked process. Stop the second run.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/gc.c | 5 +++++
 1 file changed, 5 insertions(+)
Show changes to builtin/gc.c +5 −0
diff --git a/builtin/gc.c b/builtin/gc.c
index 8d219d8..fb0cff3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 
 static int gc_before_repack(void)
 {
+	static int nr_runs;
+
+	if (nr_runs++)
+		return 0;
+
 	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
 		return error(FAILED_RUN, pack_refs_cmd.argv[0]);
 
-- 
2.1.0.rc0.78.gc0d8480
Junio C Hamano· Sep 2, 2014, 18:12 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> In the --auto code path, gc_before_repack() is called once in parent
> process then again in the forked process. Stop the second run.

Hmph. Is the true reason why this happens because we have two calls to gc_before_repack() in the main, and one is before daemonize()? Shouldn't this protection be in the caller?

Show 21 quoted lines
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/gc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 8d219d8..fb0cff3 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
>  
>  static int gc_before_repack(void)
>  {
> +	static int nr_runs;
> +
> +	if (nr_runs++)
> +		return 0;
> +
>  	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
>  		return error(FAILED_RUN, pack_refs_cmd.argv[0]);
Junio C Hamano· Sep 2, 2014, 19:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> In the --auto code path, gc_before_repack() is called once in parent
>> process then again in the forked process. Stop the second run.
>
> Hmph.  Is the true reason why this happens because we have two calls
> to gc_before_repack() in the main, and one is before daemonize()?
> Shouldn't this protection be in the caller?

Actually, I think the function already is protecting wastage of getting called twice by saying "I've done pack-refs already" and "I've done prune-reflogs already" by setting the two variables to zero. Isn't that sufficient?

What problem is this patch trying to fix?
Show 20 quoted lines
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  builtin/gc.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index 8d219d8..fb0cff3 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -261,6 +261,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
>>  
>>  static int gc_before_repack(void)
>>  {
>> +	static int nr_runs;
>> +
>> +	if (nr_runs++)
>> +		return 0;
>> +
>>  	if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
>>  		return error(FAILED_RUN, pack_refs_cmd.argv[0]);
Duy Nguyen· Sep 4, 2014, 14:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] gc --auto: do not run 'pack-refs' and 'reflog expire' twice

On Wed, Sep 3, 2014 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> In the --auto code path, gc_before_repack() is called once in parent
>>> process then again in the forked process. Stop the second run.
>>
>> Hmph.  Is the true reason why this happens because we have two calls
>> to gc_before_repack() in the main, and one is before daemonize()?
>> Shouldn't this protection be in the caller?
>
> Actually, I think the function already is protecting wastage of
> getting called twice by saying "I've done pack-refs already" and
> "I've done prune-reflogs already" by setting the two variables to
> zero.  Isn't that sufficient?
>
> What problem is this patch trying to fix?

Oops. I missed that (and should have verified the problem in the first place). Sorry for the noise.

-- 
Duy

← back to recent threads