Hi Vadim, 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/shmem.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? 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 ---------------------------(end of broadcast)--------------------------- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
