Re: logical decoding and replication of sequences

Hi,

Here's a slightly improved version of the patch, fixing a couple issues 
I failed to notice. It also addresses a couple of the issues described 
in the last message, although mostly to show what would need to be done.

1) Handle sequences dropped in the xact by calling try_relation_open, 
and just doing nothing if not found. Otherwise we'd get a failure in 
reorderbuffer, when decoding the change.

2) Fixed nextval_internal() to log the correct last_value (the one we 
write into WAL).

3) Reread the sequence state in AtEOXact_Sequences, to deal with the 
ordering issue described before. This makes (2) somewhat pointless, 
because we just read whatever is on disk at that point. But having both 
makes it easier to experiment / see what'd happen.

4) Log the stats in DefineSequence() - Without this we'd not have the 
initial sequence state in the WAL, because only nextval/setval etc. do 
the logging. The old approach (decoding the sequence tuple) does not 
have this issue.


The (3) changes the behavior in a somewhat strange way. Consider this 
case with two concurrent transactions:

T1: BEGIN;
T2: BEGIN;
T1: SELECT nextval('s') FROM generate_series(1,100) s(i);
T2: SELECT nextval('s') FROM generate_series(1,100) s(i);
T1: COMMIT;
T2: COMMIT;

The result is that both transactions have used the same sequence, and so 
will re-read the state from disk. But at that point the state is exactly 
the same, so we'll log the same thing twice.

There's a much deeper issue, though. The current patch only logs the 
sequence if the session generated WAL when incrementing the sequence 
(which happens every 32 values). But other sessions may already use 
values from this range, so consider for example this:

T1: BEGIN;
T1: SELECT nextval('s') FROM generate_series(1,100) s(i);
T2: BEGIN;
T2: SELECT nextval('s');
T2: COMMIT;
T1: ROLLBACK;

Which unfortunately means T2 already used a value, but the increment may 
not be logged at that time (or ever). This seems like a fatal issue, 
because it means we need to log *all* sequences the transaction touches, 
not just those that wrote the increment to WAL. That might still work 
for large transactions consuming many sequence values, but it's pretty 
inefficient for small OLTP transactions that only need one or two values 
from the sequence.

So I think just decoding the sequence tuples is a better solution - for 
large transactions (consuming many values from the sequence) it may be 
more expensive (i.e. send more records to replica). But I doubt that 
matters too much - it's likely negligible compared to other data for 
large transactions.


regards

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