Re: Include sequence relation support in logical replication

Hi Craig

I have added more regression test cases to the sequence replication patch with emphasis on transactions and rollback per your suggestions. I find that when a transaction is aborted with rollback, the decoder plugin will not receive the change but the sequence value will in fact advance if nextval() or setval() were called. I have also made sequence replication an optional parameter in test_decoding so other test_decoding regression test cases will not need an update due to the new sequence replication function. The sequence update in this patch will emit an wal update every 32 increment, and each update is a future value 32 increments after like it was originally, so it is no longer required getting a specific value of sequence.

Could you elaborate more on your second case where it requires a psql in an IPC::Run background session and check if regression needs more coverage on certain cases?
thank you!

Cary Huang

-------------

HighGo Software Inc. (Canada)

mailto:cary(dot)huang(at)highgo(dot)ca

http://www.highgo.ca

---- On Thu, 16 Apr 2020 09:45:06 -0700 Cary Huang <mailto:cary(dot)huang(at)highgo(dot)ca> wrote ----

Hi Craig, Andres

Thank you guys so much for your reviews and comments. Really helpful. Yes you guys are right, Sequence does not guarantee free of gaps and replicating sequence is useful for failover cases, then there will be no problem for a subscriber to get a future value 32 increments after. I will do more analysis on my end based on your comments and refine the patch with better test cases. Much appreciated of your help.

Best regards

Cary Huang

-------------

HighGo Software Inc. (Canada)

mailto:cary(dot)huang(at)highgo(dot)ca

http://www.highgo.ca

---- On Wed, 15 Apr 2020 22:18:28 -0700 Craig Ringer <mailto:craig(at)2ndquadrant(dot)com> wrote ----

On Thu, 16 Apr 2020 at 07:44, Andres Freund <mailto:andres(at)anarazel(dot)de> wrote:

> I would like to ask if you have some suggestions or ideas that can make subscriber receives the current value without the need to
>
> disabling the 32 increment behavior?

It simply shouldn't expect that to be the case.  What do you need it
for?

As far as I can tell replicating sequence values is useful to allow
failover, by ensuring all sequences will return sensible values going
forward. That does not require to now precise values.

Totally agree. Code that relies on getting specific sequence values is broken code. Alas, very common, but still broken.

Cary, by way of background a large part of why this wasn't supported by logical decoding back in 9.4 is that until the pg_catalog.pg_sequence relation was introduced in PostgreSQL 10, the sequence relfilenode intermixed a bunch of transactional and non-transactional state in a very messy way. This made it very hard to achieve sensible behaviour for logical decoding.

As it is, make sure your regression tests carefully cover the following cases, as TAP tests in src/test/recovery, probably a new module for logical decoding of sequences:

1.

* Begin txn

* Create sequence

* Call nextval() on sequence over generate_series() and discard results

* Rollback

* Issue a dummy insert+commit to some other table to force logical decoding to send something

* Ensure subscription catches up successfully

This checks that we cope with advances for a sequence that doesn't get created.

2.

 

* Begin 1st txn

* Create a sequence

* Use the sequence to populate a temp table with enough rows to ensure sequence updates are written

* Begin a 2nd txn

* Issue a dummy insert+commit to some other table to force logical decoding to send something

* Commit the 2nd txn

* Commit the 1st txn

* Wait for subscription catchup

* Check that the sequence value on the subscriber reflects the value after sequence advance, not the value at creation time

This makes sure that sequence advances are handled sensibly when they arrive for a sequence that does not yet exist in the catalogs.

You'll need to run psql in an IPC::Run background session for that. We should really have a helper for this. I'll see if I'm allowed to post the one I use for my own TAP tests to the list.