Re: Logical Replication of sequences
vignesh C <vignesh21@gmail.com>
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Doc: Add documentation for sequence synchronization.
- 55cefadde874 19 (unreleased) landed
-
Remove unused assignment in CREATE PUBLICATION grammar.
- bfb7419b0bbe 19 (unreleased) landed
-
Add seq_sync_error_count to subscription statistics.
- f6a4c498dcf6 19 (unreleased) landed
-
Fix few issues in commit 5509055d69.
- 5a4eba558aa7 19 (unreleased) landed
-
Add sequence synchronization for logical replication.
- 5509055d6956 19 (unreleased) landed
-
Add worker type argument to logical replication worker functions.
- 3e8e05596a02 19 (unreleased) landed
-
Introduce "REFRESH SEQUENCES" for subscriptions.
- f0b3573c3aac 19 (unreleased) landed
-
Refactor logical worker synchronization code into a separate file.
- 41c674d2e31e 19 (unreleased) landed
-
Standardize use of REFRESH PUBLICATION in code and messages.
- 2436b8c047ff 19 (unreleased) landed
-
Add "ALL SEQUENCES" support to publications.
- 96b378497346 19 (unreleased) landed
-
Expose sequence page LSN via pg_get_sequence_data.
- b93172ca59f4 19 (unreleased) landed
-
Resume conflict-relevant data retention automatically.
- 0d48d393d465 19 (unreleased) cited
-
Post-commit review fixes for 228c370868.
- 1f7e9ba3ac4e 19 (unreleased) cited
-
Generate GUC tables from .dat file
- 63599896545c 19 (unreleased) cited
Attachments
- v20250516-0001-Introduce-pg_sequence_state-function-for-e.patch (text/x-patch) patch v20250516-0001
- v20250516-0002-Introduce-ALL-SEQUENCES-support-for-Postgr.patch (text/x-patch) patch v20250516-0002
- v20250516-0003-Reorganize-tablesync-Code-and-Introduce-sy.patch (text/x-patch) patch v20250516-0003
- v20250516-0004-Enhance-sequence-synchronization-during-su.patch (text/x-patch) patch v20250516-0004
- v20250516-0005-Documentation-for-sequence-synchronization.patch (text/x-patch) patch v20250516-0005
On Thu, 15 May 2025 at 12:27, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Vignesh.
>
> Some minor review comments for the patches in set v20250514.
>
> ======
>
> Patch 0001.
>
> 1.1
> For function 'pg_sequence_state', the DOCS call the 2nd parameter
> 'sequence_name', but the pg_proc.dat file calls it 'seq_name'. Should
> these be made the same?
>
> ////////////////////
>
> Patch 0004.
>
> pg_sequence_state:
>
> 4.1
>
> - errmsg("sequence \"%s.%s\" does not exist",
> + errmsg("logical replication sequence \"%s.%s\" does not exist",
>
> Why isn't this change already be done in an early patch when this
> function was first implemented?
>
> ~~~
>
> copy_sequences:
>
> 4.2
>
> +/*
> + * Copy existing data of sequnces from the publisher.
> + *
>
> Typo: "sequnces"
>
> ~~~
>
> 4.3
> +{
> + int total_seq = list_length(remotesequences);
> + int curr_seq = 0;
> +
> +/*
> + * We batch synchronize multiple sequences per transaction, because the
> + * alternative of synchronizing each sequence individually incurs overhead of
> + * starting and committing transactions repeatedly. On the other hand, we want
> + * to avoid keeping this batch transaction open for extended periods so it is
> + * currently limited to 100 sequences per batch.
> + */
> +#define MAX_SEQUENCES_SYNC_PER_BATCH 100
>
> Wrong indent for block comment.
>
> ~~~
>
> 4.4
> + if (res->status != WALRCV_OK_TUPLES)
> + ereport(ERROR,
> + errcode(ERRCODE_CONNECTION_FAILURE),
> + errmsg("could not receive list of sequences information from the
> publisher: %s",
> + res->err));
>
> Should this say /sequences information/sequence information/
>
> ~~~
>
> 4.5
> + ereport(LOG,
> + errmsg("Logical replication sequence synchronization - total
> unsynchronized: %d, attempted in this batch: %d; succeeded in this
> batch: %d; mismatched in this batch: %d for subscription: \"%s\"",
> + total_seq, batch_seq_count, batch_success_count,
> + batch_mismatch_count, get_subscription_name(subid, false)));
> +
>
>
> This errmsg seems backwards. I think it should be expressed like the
> other one immediately above. Also I think the information can be made
> shorter -- e.g. no need to say "in this batch" multiple times.
>
> SUGGESTION
> "Logical replication sequence synchronization for subscription \"%s\"
> - total unsynchronized: %d; batch #%d = %d attempted, %d succeeded, %d
> mismatched"
Thanks for the comments, these are handled in the attached v20250516
version patch.
Regards,
Vignesh