Alvaro Herrera <[email protected]> wrote: > On 2026-Sep-18, Antonin Houska wrote: > > > Maybe I miss the point, but what's wrong about modifying the existing loop > > that inverts the meaning of the ->xip array > > > > /* > > * snapbuild.c builds transactions in an "inverted" manner, which means > > it > > * stores committed transactions in ->xip, not ones in progress. Build a > > * classical snapshot by marking all non-committed transactions as > > * in-progress. This can be expensive. > > */ > > for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) > > { > > ... > > } > > > > by calling XactLockTableWait() for each XID we find in the array (i.e. each > > committed transaction)? > > Ah, you mean something like the attached quick POC? This does pass the > two tests that Rui wrote, also attached. (I didn't test Zhijie's, which > AFAICT is written to pass with the bug and fail without it.)
Yes, I mean checking if those transactions have really ended. Regarding [1], perhaps the idea is to avoid calling XactLockTableWait() if the transaction is no longer in procarray. I'm not sure it's a problem to call that function (possibly many times) for transactions that are no longer running. (GetRunningTransactionData() is not free either.) And regarding the deadlock with synchronous replica, my understanding is that we avoid it by waiting in SnapBuildInitialSnapshot() instead of in SnapBuildBuildSnapshot(). [1] https://www.postgresql.org/message-id/CAHWVJhHXyLtS-8mdL9WhEWfsERb%3DFN7JdPD0GYAXgTmCnqbYGw%40mail.gmail.com -- Antonin Houska Web: https://www.cybertec-postgresql.com
