On Wed, Jan 10, 2024 at 02:18:52PM -0800, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > But that requires us to tweak production code (albeit at a negligible > > cost) in order to appease LSan in this narrow circumstance. Another > > approach is to simply run these expected-to-fail `index-pack` > > invocations with `--threads=1` so that we bypass the above issue > > entirely. > > But of course, multi-threaded operation that production folks use > will not be tested at all with the alternative. Just the ones that we expect to fail *and* are in test scripts which are marked as leak-free. > > The downside of that approach is that the test doesn't match our > > production code as well as it did before (where we might have run those > > same `index-pack` invocations with >1 thread, depending on how many CPUs > > the testing machine has). The risk there is that we might miss a > > regression that would leave us in an inconsistent state. But that feels > > rather unlikely in practice, and there are many other tests related to > > `index-pack` in the suite. > > As long as "make sure we spawn all of them atmically" has negligible > downside, I'd rather take that approach. Buying predictability with > minimum cost is quite attractive. Like I said earlier, I have no strong preference between either approach. If you want to pick up Peff's patch instead of these five, that is fine with me :-). Thanks, Taylor
