Re: SSI: A patch for a Serializability violation

Hi Andrey,

After looking into this further, I realized that to solve the issue you
raised regarding imported snapshots, we can look for the actual sxact that
exported the snapshot. We can then use its lastCommitBeforeSnapshot as the
importer's lastCommitBeforeSnapshot, provided the importing transaction is
read-write.

If the importer is read-only, a higher value does not matter and would only
result in false positives or delayed resource reclamation. For a read-write
importer, using the exporter's value vs. PredXact->LastSxactCommitSeqNo
only matters with your patch to fix the consistency issue, as it could
otherwise cause missed conflicts.

In other words, combining the attached patch with yours resolves the
inconsistency. Without your patch, the attached patch makes no difference.

Please let me know if you agree. I have attached the patch to illustrate
this approach. I am still testing this specific scenario and will share the
tests in a subsequent patch.

Regards,
Vaijayanti Bharadwaj

On Mon, Aug 31, 2026 at 3:35 PM Vaijayanti Bharadwaj <
vaijayanti(dot)bharadwaj(at)enterprisedb(dot)com> wrote:

> 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
>>
>>