Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

On Thu, 26 Jan 2023 at 19:53, Melih Mutlu <m.melihmutlu@gmail.com> wrote:
>
> Hi Shveta,
>
> Thanks for reviewing.
>
> shveta malik <shveta.malik@gmail.com>, 25 Oca 2023 Çar, 16:02 tarihinde şunu yazdı:
>>
>> On Mon, Jan 23, 2023 at 6:30 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
>> --I see initial data copied, but new catalog columns srrelslotname
>> and srreloriginname are not updated:
>> postgres=# select sublastusedid from pg_subscription;
>>  sublastusedid
>> ---------------
>>              2
>>
>> postgres=# select * from pg_subscription_rel;
>>  srsubid | srrelid | srsubstate | srsublsn  | srrelslotname | srreloriginname
>> ---------+---------+------------+-----------+---------------+-----------------
>>    16409 |   16384 | r          | 0/15219E0 |               |
>>    16409 |   16389 | r          | 0/15219E0 |               |
>>    16409 |   16396 | r          | 0/15219E0 |               |
>>
>> When are these supposed to be updated? I thought the slotname created
>> will be updated here. Am I missing something here?
>
>
> If a relation is currently being synced by a tablesync worker and uses a replication slot/origin for that operation,
thensrrelslotname and srreloriginname fields will have values. 
> When a relation is done with its replication slot/origin, their info gets removed from related catalog row, so that
slot/origincan be reused for another table or dropped if not needed anymore. 
> In your case, all relations are in READY state so it's expected that srrelslotname and srreloriginname are empty.
READYrelations do not need a replication slot/origin anymore. 
>
> Tables are probably synced so quickly that you're missing the moments when a tablesync worker copies a relation and
storesits rep. slot/origin in the catalog. 
> If initial sync is long enough, then you should be able to see the columns get updated. I follow [1] to make it
longerand test if the patch really updates the catalog. 
>
>
>>
>> Also the v8 patch does not apply on HEAD, giving merge conflicts.
>
>
> Rebased and resolved conflicts. Please check the new version

CFBot shows some compilation errors as in [1], please post an updated
version for the same:
[14:38:38.392] [827/1808] Compiling C object
src/backend/postgres_lib.a.p/replication_logical_tablesync.c.o
[14:38:38.392] ../src/backend/replication/logical/tablesync.c: In
function ‘LogicalRepSyncTableStart’:
[14:38:38.392] ../src/backend/replication/logical/tablesync.c:1629:3:
warning: implicit declaration of function ‘walrcv_slot_snapshot’
[-Wimplicit-function-declaration]
[14:38:38.392] 1629 | walrcv_slot_snapshot(LogRepWorkerWalRcvConn,
slotname, &options, origin_startpos);
[14:38:38.392] | ^~~~~~~~~~~~~~~~~~~~

[14:38:45.125] FAILED: src/backend/postgres
[14:38:45.125] cc @src/backend/postgres.rsp
[14:38:45.125] /usr/bin/ld:
src/backend/postgres_lib.a.p/replication_logical_tablesync.c.o: in
function `LogicalRepSyncTableStart':
[14:38:45.125] /tmp/cirrus-ci-build/build/../src/backend/replication/logical/tablesync.c:1629:
undefined reference to `walrcv_slot_snapshot'
[14:38:45.125] collect2: error: ld returned 1 exit status

[1] - https://cirrus-ci.com/task/4897131543134208?logs=build#L1236

Regards,
Vignesh