Re: sequences vs. synchronous replication

On 1/25/22 10:18, Peter Eisentraut wrote:
>
> On 15.01.22 23:57, Tomas Vondra wrote:
>>> This approach (and also my previous proposal) seems to assume that
>>> the value returned from nextval() should not be used until the
>>> transaction executing that nextval() has been committed successfully.
>>> But I'm not sure how many applications follow this assumption. Some
>>> application might use the return value of nextval() instantly before
>>> issuing commit command. Some might use the return value of nextval()
>>> executed in rollbacked transaction.
>>>
>>
>> IMO any application that assumes data from uncommitted transactions is
>> outright broken and we should not try to fix that because it's quite
>> futile (and likely will affect well-behaving applications).
>>
>> The issue I'm trying to fix in this thread is much narrower - we don't
>> actually meet the guarantees for committed transactions (that only did
>> nextval without generating any WAL).
>
> The wording in the SQL standard is:
>
> "Changes to the current base value of a sequence generator are not
> controlled by SQL-transactions; therefore, commits and rollbacks of
> SQL-transactions have no effect on the current base value of a sequence
> generator."
>
> This implies the well-known behavior that consuming a sequence value is
> not rolled back.  But it also appears to imply that committing a
> transaction has no impact on the validity of a sequence value produced
> during that transaction.  In other words, this appears to imply that
> making use of a sequence value produced in a rolled-back transaction is
> valid.
>
> A very strict reading of this would seem to imply that every single
> nextval() call needs to be flushed to WAL immediately, which is of
> course impractical.

I'm not an expert in reading standards, but I'd not interpret it that
way. I think it simply says the sequence must not go back, no matter
what happened to the transaction.

IMO interpreting this as "must not lose any increments from uncommitted
transactions" is maybe a bit too strict, and as you point out it's also
impractical because it'd mean calling nextval() repeatedly flushes WAL
all the time. Not great for batch loads, for example.

I don't think we need to flush WAL for every nextval() call, if we don't
write WAL for every increment - I think we still can batch WAL for 32
increments just like we do now (AFAICS that'd not contradict even this
quite strict interpretation of the standard).

OTOH the flush would have to happen immediately, we can't delay that
until the end of the transaction. Which is going to affect even cases
that generate WAL for other reasons (e.g. doing insert), which was
entirely unaffected by the previous patches.

And the flush would have to happen even for sessions that didn't write
WAL (which was what started this thread) - we could use page LSN and
flush only to that (so we'd flush once and then it'd be noop until the
sequence increments 32-times and writes another WAL record).

Of course, it's not enough to just flush WAL, we have to wait for the
sync replica too :-(

I don't have any benchmark results quantifying this yet, but I'll do
some tests in the next day or two. But my expectation is this is going
to be pretty expensive, and considering how concerned we were about
affecting current workloads, making the impact worse seems wrong.

My opinion is we should focus on fixing this given the current (weaker)
interpretation of the standard, i.e. accepting the loss of increments
observed only by uncommitted transactions. The page LSN patch seems like
the best way to do that so far.

We may try reworking this to provide the stronger guarantees (i.e. not
losing even increments from uncommitted transactions) in the future, of
course. But considering (a) we're not sure that's really what the SQL
standard requires, (b) no one complained about that in years, and (c)
it's going to make sequences way more expensive, I doubt that's really
desirable.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company