Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Hou, Zhijie/侯 志杰 <houzj.fnst@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2024-08-12T13:06:26Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Doc: Add documentation for sequence synchronization.

  2. Remove unused assignment in CREATE PUBLICATION grammar.

  3. Add seq_sync_error_count to subscription statistics.

  4. Fix few issues in commit 5509055d69.

  5. Add sequence synchronization for logical replication.

  6. Add worker type argument to logical replication worker functions.

  7. Introduce "REFRESH SEQUENCES" for subscriptions.

  8. Refactor logical worker synchronization code into a separate file.

  9. Standardize use of REFRESH PUBLICATION in code and messages.

  10. Add "ALL SEQUENCES" support to publications.

  11. Expose sequence page LSN via pg_get_sequence_data.

  12. Resume conflict-relevant data retention automatically.

  13. Post-commit review fixes for 228c370868.

  14. Generate GUC tables from .dat file

Attachments

On Mon, 12 Aug 2024 at 08:50, Peter Smith <smithpb2250@gmail.com> wrote:
>
> ~~~
>
> 3.
> I am not sure that it was an improvement to move the
> process_syncing_sequences_for_apply() function into the
> sequencesync.c. Calling the sequence code from the tablesync code
> still looks strange. OTOH, I see why you don't want to leave it in
> tablesync.c.
>
> Perhaps it would be better to refactor/move all following functions
> back to the (apply) worker.c instead:
> - process_syncing_relations
> - process_syncing_sequences_for_apply(void)
> - process_syncing_tables_for_apply(void)
>
> Actually, now that there are 2 kinds of 'sync' workers, maybe you
> should introduce a new module (e.g. 'commonsync.c' or
> 'syncworker.c...), where you can put functions such as
> process_syncing_relations() plus any other code common to both
> tablesync and sequencesync. That might make more sense then having one
> call to the other.

I created syncutils.c to consolidate code that supports worker
synchronization, table synchronization, and sequence synchronization.
While it may not align exactly with your suggestion, I included
functions like finish_sync_worker, invalidate_syncing_relation_states,
FetchRelationStates, and process_syncing_relations in this new file. I
believe this organization will make the code easier to review.

The rest of the comments are also fixed in the attached v20240812
version patch attached.

Regards,
Vignesh