Horton

Re: logical decoding and replication of sequences

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Petr Jelinek <petr.jelinek@enterprisedb.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-23T11:50:34Z
Lists: pgsql-hackers 
On Tue, Mar 22, 2022 at 5:41 PM Petr Jelinek
<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.

On looking a bit more closely, I think I see some more fundamental
problems here:

*  Don't we need some syncing mechanism between apply worker and
sequence sync worker so that apply worker skips the sequence changes
till the sync worker is finished, otherwise, there is a risk of one
overriding the values of the other?

* 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.

* Don't we need explicit privilege checking before applying sequence
data as we do in commit a2ab9c06ea15fbcb2bfde570986a06b37f52bcca for
tables?


-- 
With Regards,
Amit Kapila.

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.