Re: Proposal for CSN based snapshots

On 2015-07-24 14:21:39 -0400, Robert Haas wrote:
> - Mixing synchronous_commit=on and synchronous_commit=off won't work
> as well, because if the LSN ordering of commit records matches the
> order in which transactions become visible, then an async-commit
> transaction can't become visible before a later sync-commit
> transaction. I expect we might just decide we can live with this, but
> it's worth discussing in case people feel otherwise.

I'm not following this anymore. Even though a couple months back I
apparently did. Heikki's description of the problem was:

On 2014-05-30 17:59:23 +0300, Heikki Linnakangas wrote:
> One thorny issue came up in discussions with other hackers on this in PGCon:
>
> When a transaction is committed asynchronously, it becomes visible to other
> backends before the commit WAL record is flushed. With CSN-based snapshots,
> the order that transactions become visible is always based on the LSNs of
> the WAL records. This is a problem when there is a mix of synchronous and
> asynchronous commits:
>
> If transaction A commits synchronously with commit LSN 1, and transaction B
> commits asynchronously with commit LSN 2, B cannot become visible before A.
> And we cannot acknowledge B as committed to the client until it's visible to
> other transactions. That means that B will have to wait for A's commit
> record to be flushed to disk, before it can return, even though it was an
> asynchronous commit.

Afacs the xlog insertion order isn't changed relevantly by the patch?

Right now the order for sync commit = on is:

XactLogCommitRecord();
XLogFlush();
TransactionIdCommitTree();
ProcArrayEndTransaction();

and for sync commit = off it's:
XactLogCommitRecord();
TransactionIdAsyncCommitTree();
ProcArrayEndTransaction();

with the CSN patch it's:

sync on:
TransactionIdSetCommitting();
XactLogCommitRecord();
XLogFlush(XactLastRecEnd);
TransactionIdCommitTree();

sync off:
TransactionIdSetCommitting();
XactLogCommitRecord();
TransactionIdCommitTree();

How does this cause the above described problem? Yes, mixing sync/async
commits can give somewhat weird visibility semantics, but that's not
really new, is it?

Regards,

Andres