Re: Logical Replication of sequences

On Mon, 15 Sept 2025 at 14:36, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 8 Sept 2025 at 12:05, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> >
> >
> > On Sep 8, 2025, at 14:00, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> >
> >
> > 1 - 0001
> > ```
> > diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
> > index 2c220b60749..c8adddbfa31 100644
> > --- a/src/test/regress/sql/sequence.sql
> > +++ b/src/test/regress/sql/sequence.sql
> > @@ -414,6 +414,6 @@ SELECT nextval('test_seq1');
> > SELECT nextval('test_seq1');
> >
> > -- pg_get_sequence_data
> > -SELECT * FROM pg_get_sequence_data('test_seq1');
> > +SELECT last_value, is_called, log_cnt, page_lsn <= pg_current_wal_lsn() as lsn FROM pg_get_sequence_data('test_seq1');
> >
> > DROP SEQUENCE test_seq1;
> > ```
> >
> > As it shows log_cnt now, after calling pg_get_sequence_data(), I suggest add 8 nextval(), so that sequence goes to 11, and log_cnt should become to 22.
> >
> >
> > Could you please explain the reason you’d like this to be done?
> >
> >
> > Because log_cnt is newly exposed, we want to verify its value in the test. When I first time ran the test code, I saw initial value of log_cnt was 32, then I thought log_cnt might get decreased if I ran nextval() again, but it didn’t. Only after I ran 10 (cache size) more nextval(), log_cnt got decreased by 10 to 22. The test code is a place for people to look for expected behavior. So I think adding more nextval() to verify and show the change of log_cnt is helpful.
>
> This is addressed in the attached patch, also rebased the patch
> because of recent commits.

Hi Vignesh,

Here are some more review comments:

For patch 0006:

1. Spelling mistake in:
+ appendStringInfo(combined_error_detail, "Insufficent permission for
sequence(s): (%s).",
+ insuffperm_seqs->data);

Insufficent -> Insufficient

2. Spelling mistake in:
+ ereport(LOG,
+ errmsg("logical replication sequence synchronization for
subscription \"%s\" - batch #%d = %d attempted, %d succeeded, %d
skipped, %d mismatched, %d insufficient pemission, %d missing, ",
+ MySubscription->name, (current_index /
MAX_SEQUENCES_SYNC_PER_BATCH) + 1, batch_size,
+ batch_succeeded_count, batch_skipped_count,
batch_mismatched_count, batch_insuffperm_count,
+ batch_size - (batch_succeeded_count + batch_skipped_count +
batch_mismatched_count + batch_insuffperm_count)));

pemission -> permission

3. I ran the ALTER SUBSCRIPTION .. REFRESH PUBLICATION command and
DROP SEQUENCE command and got a warning for "leaked hash_seq_search
scan". Is it expected?

2025-09-17 19:06:48.663 IST [2995060] LOG: logical replication
sequence synchronization worker for subscription "sub1" has started
2025-09-17 19:06:48.677 IST [2995060] LOG: logical replication
sequence synchronization for subscription "sub1" - total
unsynchronized: 0
2025-09-17 19:06:48.677 IST [2995060] WARNING: leaked hash_seq_search
scan for hash table 0x62b0a61d3450
2025-09-17 19:06:48.677 IST [2995060] LOG: logical replication
sequence synchronization worker for subscription "sub1" has finished

Steps to reproduce:
1. create publication for ALL SEQUENCES and create a subscription for
the publication on another node.
2. create sequence s1 both on publisher and subscriber.
3. Attach gdb on a psql terminal and add breakpoint at "line of
function call to AddSubscriptionRelState" inside function
AlterSubscription_refresh and run ALTER PUBLICATION .. REFRESH
PUBLICATION command on psql terminal
4. DROP sequence s1 on another terminal(for subscriber)
5. Continue the gdb
We will get the above warning.

Thanks,
Shlok Kyal