Re: SSI: A patch for a Serializability violation

My fix came from a different direction. I first hit shared memory
exhaustion from summarized predicate locks never being freed, and while
fixing that I considered adding finishedBefore to PREDICATELOCK. Once I had
the field, I noticed the check in CheckTargetForConflictsIn() can use
finishedBefore which is not populated for OldCommittedSxact, so I reused it
for both.

But you have a very good point about imported snapshots.
GetSerializableTransactionSnapshotInt() sets SeqNo.lastCommitBeforeSnapshot
from the live PredXact->LastSxactCommitSeqNo at import time, while
sxact->xmin comes from the imported snapshot, and nothing reconciles the
two. lastCommitBeforeSnapshot can end up too large. Like you said, a reader
that commits between export and import loses its rw-edge. That is a missed
conflict rather than a false positive.

I think its a super rare case, it needs a read-write SERIALIZABLE
transaction importing a snapshot, pg_dump uses REPEATABLE READ, READ ONLY
for any connection that imports, and logical replication tablesync is also
REPEATABLE READ. But an application can do it.

I think your approach would work for the above case with a small addition:
flag sxacts that imported a snapshot, and in
PredicateLockIsForOverlappingTransaction() treat every summarized lock as
overlapping for those. It is pessimistic, but only for a rare case.

So far, I have only looked into PredicateLockIsForOverlappingTransaction(),
and have yet to evaluate other uses of lastCommitBeforeSnapshot for the
case of importing a snapshot. But if you have done so, I would like to
understand that.

Thank you!
Vaijayanti Bharadwaj

On Mon, Aug 31, 2026 at 1:37 PM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:

> Hi Vaijayanti,
>
> Jacob reported what looks like the same bug earlier [0], and I sent a
> tentative commitSeqNo-based fix in that thread.
>
> Your patch made me notice a case which we did not consider there: imported
> serializable snapshots. lastCommitBeforeSnapshot is recorded when the
> snapshot is imported, while finishedBefore is compared with the imported
> snapshot's xmin. The two approaches can therefore disagree about a reader
> which commits between export and import. Your patch preserves the existing
> non-summarized behavior in this case; mine does not.
>
> Was this case part of your reasoning? It seems worth writing explicitly
> the
> expected behavior here.
>
> Thank you!
>
> Best regards, Andrey Borodin.
>
>
> [0]
> https://postgr.es/m/CA%2BCOZaCtK%3DUQbeQwdAoRw27J%2B58bJBC%2ByNHP4OH2%2By_t2UtFAg%40mail.gmail.com
>
>