Horton

Re: logical decoding and replication of sequences

Petr Jelinek <petr.jelinek@enterprisedb.com>

View thread View on postgr.es
From: Petr Jelinek <petr.jelinek@enterprisedb.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-23T12:46:29Z
Lists: pgsql-hackers 
> On 23. 3. 2022, at 12:50, Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Tue, Mar 22, 2022 at 5:41 PM Petr Jelinek
> <petr.jelinek@enterprisedb.com <mailto:petr.jelinek@enterprisedb.com>> wrote:
>> 
>>> On 22. 3. 2022, at 13:09, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> 
>>> On Mon, Mar 21, 2022 at 4:25 AM Tomas Vondra
>>> <tomas.vondra@enterprisedb.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Attached is a rebased patch, addressing most of the remaining issues.
>>>> 
>>> 
>>> It appears that on the apply side, the patch always creates a new
>>> relfilenode irrespective of whether the sequence message is
>>> transactional or not. Is it required to create a new relfilenode for
>>> non-transactional messages? If not that could be costly?
>>> 
>> 
>> 
>> That's a good catch, I think we should just write the page in the non-transactional case, no need to mess with relnodes.
>> 
> 
> What if the current node has also incremented from the existing
> sequence? Basically, how will we deal with conflicts? It seems we will
> overwrite the actions done on the existing node which means sequence
> values can go back.
> 


I think this is perfectly acceptable behavior, we are replicating state from upstream, not reconciling state on downstream.

You can't really use the builtin sequences to implement distributed sequence via replication. If user wants to write to both nodes they should not replicate the sequence value and instead offset the sequence on each node so they produce different ranges, that's quite common approach. One day we might want revisit adding support for custom sequence AMs.


> * Currently, the patch uses one sync worker per sequence. It seems to
> be a waste of resources considering apart from one additional process,
> we need origin/slot to sync each sequence.
> 


This is indeed wasteful but not something that I'd consider blocker for the patch personally.

--  
Petr

Commits

  1. Fix cache invalidation bug in recovery_prefetch.

  2. Revert "Logical decoding of sequences"

  3. Minor improvements in sequence decoding code and docs

  4. Handle sequences in preprocess_pubobj_list

  5. Update tab-completion for CREATE PUBLICATION with sequences

  6. Add decoding of sequences to built-in replication

  7. Stabilize test_decoding touching with sequences

  8. Call ReorderBufferProcessXid from sequence_decode

  9. Add decoding of sequences to test_decoding

  10. Replace Test::More plans with done_testing

  11. Logical decoding of sequences

  12. Respect permissions within logical replication.