Re: [PATCH 00/10] reftable: fix reload with active iterators

From: karthik nayak <karthik.188@gmail.com>
Date: Aug 22, 2024, 08:13 UTC
Message-ID: <CAOLa=ZR+bbYrYrZ79zPY3eYWQRQo97NfmcJXS=-nD842g7=XiQ@mail.gmail.com>
In-Reply-To: <cover.1724080006.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 7 quoted lines

> Hi,
>
> as reported by Peff in [1], the reftable backend fails in t5616 in a
> racy manner. The cause of this is that git-maintenance(1) processes leak
> into subsequent tests and perform concurrent compaction of the stack.
> The reftable backend is expected to handle this gracefully, but doesn't.
>

I've not looked into the recent changes made to `git-maintenance(1)` but my understanding is that, now, `git-maintenance(1)` can detach itself without relying on `git-gc(1)`'s detach.

Show 5 quoted lines

> The issue can surface whenever reloading the reftable stack while an
> iterator is active. In case some of the tables got compacted, we will
> end up closing them and thus the iterator now tries to use those closed
> tables.
>

Make sense till here.

> This patch series fixes that issue by starting to refcount the readers.

Not sure what you mean by 'refcount' here though.

Show 11 quoted lines

> Each iterator will bump the refcount, thus avoiding the problem. While
> at it, I also found a second issue where we segfault when reloading a
> table fails while reusing one of the table readers. In this scenario, we
> would end up releasing the reader of the stack itself, even though it
> would still be used by it.
>
> This patch series addresses those issues by making the reftable reader
> refcounted. The first 6 patches do some simplifications of code which is
> close. The remaining 4 ones introduce refcounting and wire it up as
> required.
>

Perhaps it becomes more clearer as I read through the patches.

[snip]