Re: [PATCH 1/6] maintenance: add get_random_minute()

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 7, 2023, 21:20 UTC
Message-ID: <ZNFgIyuhlNd8I9Y2@nand.local>
In-Reply-To: <fefdaa9457948ee5302e7cbfaae250e0b589d752.1691434300.git.gitgitgadget@gmail.com>

On Mon, Aug 07, 2023 at 06:51:35PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 21 quoted lines

> diff --git a/builtin/gc.c b/builtin/gc.c
> index f3942188a61..66a972bc292 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1708,6 +1708,23 @@ static int get_schedule_cmd(const char **cmd, int *is_available)
>  	return 1;
>  }
>
> +MAYBE_UNUSED
> +static int get_random_minute(void)
> +{
> +	static int random_initialized = 0;
> +
> +	/* Use a static value when under tests. */
> +	if (!getenv("GIT_TEST_MAINTENANCE_SCHEDULER"))
> +		return 13;
> +
> +	if (!random_initialized) {
> +		srand((unsigned int)getpid());
> +		random_initialized = 1;
> +	}

I was wondering where else we call srand() within Git, and it looks like the only other spot is in `lock_file_timeout()`.

I doubt it, but is there a chance that that code depends on only calling srand() once? I think the answer is "no", since we only use rand() within that function to generate a random-ish backoff period, so I think the repeatability of it doesn't matter all that much.

So I think this is kind of outside the scope of your series, but I wonder if we should have a git_rand() that automatically initializes the PRNG with the value of getpid()? Then multiple callers can grab random values at will without reinitializing the PRNG.

Thanks, Taylor