Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Jun 27, 2022 at 11:53:49PM -0600, Tracey Emery wrote:
> > On Mon, Jun 27, 2022 at 11:12:30PM +0200, Stefan Sperling wrote:
> > > This patch offloads parts of findtwixt() into got-read-pack if possible.
> > > This significantly speeds up commit coloring if a large pack file is
> > > available.
> > > 
> > > I have one some light testing, packing and got send still work as
> > > expected for me. More testing would be welcome.
> > > 
> > > ok?
> > > 
> > 
> > Ok with a couple of things below. If I've missed anything, and I
> > probably have, because my normal daily-lack-of-sleep-pattern has now
> > resumed, we can fix it in the tree.
>
> There was a bug in pinned pack handling where we could end up
> sending a commit painting requests to a non-pinned got-read-pack
> process, resulting in an unexpected privsep message error.
> I found this during 'got send'.
> 
> This bug was fixed at the spot where we move a cached pack to the
> front of the cache.
> 
> +		if (repo->pinned_pack == 0)
> +			repo->pinned_pack = i;
> 
> becomes:
> 
> +		if (repo->pinned_pack == 0)
> +			repo->pinned_pack = i;
> +		else if (repo->pinned_pack == i)
> +			repo->pinned_pack = 0;

I went thru the diff a couple of times, can't spot anything wrong.  I
did a few tests with `got send' and `gotadmin pack -a' other than
running the regress suite a couple of times with and without
GOT_TEST_PACK and they were successful.

fwiw, ok op

To be fair thought, I couldn't spot this bug, neither by reading the
diff nor testing it.

> And we now store the PID of the pinned got-read-pack helper.
> The PID isn't really used for anything, but was very helpful
> for debugging and won't hurt, so I decided to leave it in.

agreed, i wished to have pids around when debugging the 'bad offset in
pack file' bug some time ago, so i can see how useful it is :)