Re: [PATCH v3 1/8] reftable/stack: do not overwrite errors when compacting

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 15, 2024, 07:43 UTC
Message-ID: <Zc3AsEu0FYzU6tMX@tanuki>
In-Reply-To: <CAOw_e7b72HVQob_hiV0gtRhGWsb=rz40WL=oaV63t7oOmEA-mw@mail.gmail.com>

On Wed, Feb 14, 2024 at 04:12:54PM +0100, Han-Wen Nienhuys wrote:

> Good catch!
> 
> Sorry for messing this up.

Nothing to be sorry about, bugs happen to all of us.

Show 13 quoted lines

> > In the worst case,
> > this can lead to a compacted stack that is missing records.
> 
> Yeah, that would be an insidious corruption. Have you considered
> writing a test to reproduce this (and thus verify that the fix really
> fixes the problem?)
> 
> I think it wouldn't be too difficult: you could create a custom
> blocksource wrapper that returns I/O error on the Nth read, and then
> create a reftable with two ref blocks (could just be 2 records if you
> use a small blocksize and a large refname) and two log blocks.  Merge
> that with an empty table, and see if the compacted result is what you
> got in. Loop over N to get coverage for all error paths.

Ah, that's a feasible way to write such a test indeed. I may come back to it in the future and will add it to our backlog. Thanks!

Patrick