> I am trying to understand why GetSnapshotData() needs to acquire the
> SInval spinlock before it calls ReadNewTransactionId, rather than after.
> I see that you made it do so --- in the commit at
>
http://www.ca.postgresql.org/cgi/cvsweb.cgi/pgsql/src/backend/storage/ipc/sh
mem.c.diff?r1=1.41&r2=1.42
> but I don't understand why the loss of concurrency is "necessary".
> Since we are going to treat all xids >= xmax as in-progress anyway,
> what's wrong with reading xmax before we acquire the SInval lock?
AFAIR, I made so to prevent following:
1. Tx Old is running.
2. Tx S reads new transaction ID in GetSnapshotData() and swapped away before SInval acquired.
3. Tx New gets new transaction ID, makes changes and commits.
4. Tx Old changes some row R changed by Tx New and commits.
5. Tx S gets snapshot data and now sees R changed by *both* Tx Old and Tx New *but* does not see *other* changes made
byTx New => Tx S reads unconsistent data.
---------
As for issue below - I don't remember why I decided that
it's not important and will need in some time to remember.
> Also, it seems to me that in GetNewTransactionId(), it's important
> for MyProc->xid to be set before releasing XidGenLockId, not after.
> Otherwise there is a narrow window for failure:
>
> 1. Process A calls GetNewTransactionId. It allocates an xid of, say,
> 1234, and increments nextXid to 1235. Just after releasing the
> XidGenLock spinlock, but before it can set its MyProc->xid, control
> swaps away from it.
>
> 2. Process B gets to run. It runs GetSnapshotData. It sees nextXid =
> 1235, and it does not see xid = 1234 in any backend's proc->xid.
> Therefore, B will assume xid 1234 has already terminated, when it
> hasn't.
>
> Isn't this broken? The problem would be avoided if
> GetNewTransactionId
> sets MyProc->xid before releasing the spinlock, since then after
> GetSnapshotData has called ReadNewTransactionId, we know that
> all older
> XIDs that are still active are recorded in proc structures.
>
> Comments?
>
> regards, tom lane
>