Re: Minimal logical decoding on standbys

On 3/22/21 4:56 PM, Drouvot, Bertrand wrote:
>
>
> On 3/22/21 3:57 PM, Drouvot, Bertrand wrote:
>>
>>
>> On 3/22/21 3:10 PM, Fabrízio de Royes Mello wrote:
>>>
>>> On Thu, Mar 18, 2021 at 5:34 AM Drouvot, Bertrand
>>> <bdrouvot(at)amazon(dot)com <mailto:bdrouvot(at)amazon(dot)com>> wrote:
>>> >
>>> > Thanks!
>>> >
>>> > Just made minor changes to make it compiling again on current
>>> master (mainly had to take care of
>>> ResolveRecoveryConflictWithSnapshotFullXid() that has been
>>> introduced in e5d8a99903).
>>> >
>>> > Please find enclosed the new patch version that currently passes
>>> "make check" and the 2 associated TAP tests.
>>> >
>>>
>>> Unfortunately it still not applying to the current master:
>>>
>>> $ git am ~/Downloads/v10-000*.patch
>>> Applying: Allow logical decoding on standby.
>>> Applying: Add info in WAL records in preparation for logical slot
>>> conflict handling.
>>> error: patch failed: src/backend/access/nbtree/nbtpage.c:32
>>> error: src/backend/access/nbtree/nbtpage.c: patch does not apply
>>> Patch failed at 0002 Add info in WAL records in preparation for
>>> logical slot conflict handling.
>>> hint: Use 'git am --show-current-patch' to see the failed patch
>>> When you have resolved this problem, run "git am --continue".
>>> If you prefer to skip this patch, run "git am --skip" instead.
>>> To restore the original branch and stop patching, run "git am --abort".
>>
>> oh Indeed, it's moving so fast!
>>
>> Let me rebase it (It was already my plan to do so as I have observed
>> during the week end that the
>> v7-0003-Handle-logical-slot-conflicts-on-standby.patch
>> <https://www.postgresql.org/message-id/attachment/117031/v7-0003-Handle-logical-slot-conflicts-on-standby.patch>
>> introduced incorrect changes (that should not be there at all in
>> ReplicationSlotReserveWal()) that have been kept in v8, v9 and v10.
>>
> please find enclosed the rebase version, that also contains the fix
> for ReplicationSlotReserveWal() mentioned above.
>
> Going back to looking at the whole thread.
>
I have one remark regarding the conflicts:

The logical slots are dropped if a conflict is detected.

But, if the slot is not active before being dropped (say wal_level is
changed to  < logical on master and a logical slot is not active on the
standby) then its corresponding
pg_stat_database_conflicts.confl_logicalslot is not incremented (as it
would be incremented "only" during the cancel of an "active" backend).

I think, it should be incremented in all the cases (active or not), what
do you think?

I updated the patch to handle this scenario (see the new
pgstat_send_droplogicalslot() in
v12-0003-Handle-logical-slot-conflicts-on-standby.patch).

I also added more tests in 023_standby_logical_decoding_conflicts.pl to
verify that pg_stat_database_conflicts.confl_logicalslot is updated as
expected (see check_confl_logicalslot() in
v12-0004-New-TAP-test-for-logical-decoding-on-standby.patch).

Except this remark and the associated changes, then it looks good to me.

Bertrand