Re: Minimal logical decoding on standbys

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Amit Khandekar <amitdkhan.pg@gmail.com>, fabriziomello@gmail.com, tushar <tushar.ahuja@enterprisedb.com>, Rahila Syed <rahila.syed@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-24T05:20:20Z
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. Reduce the log level in 035_standby_logical_decoding.pl.

  2. 035_standby_logical_decoding: Add missing waits for replication

  3. For cascading replication, wake physical and logical walsenders separately

  4. Handle logical slot conflicts on standby

  5. Support invalidating replication slots due to horizon and wal_level

  6. Prevent use of invalidated logical slot in CreateDecodingContext()

  7. Replace replication slot's invalidated_at LSN with an enum

  8. Pass down table relation into more index relation functions

  9. Assert only valid flag bits are passed to visibilitymap_set()

  10. Remove unused _bt_delitems_delete() argument.

  11. Add xl_btree_delete optimization.

Hi,

On 1/24/23 1:46 AM, Andres Freund wrote:
> Hi,
> 
> On 2023-01-19 10:43:27 +0100, Drouvot, Bertrand wrote:
>>>> With a reload in place in my testing, now I notice that the catalog_xmin
>>>> is updated on the primary physical slot after logical slots invalidation
>>>> when reloading hot_standby_feedback from "off" to "on".
>>>>
>>>> This is not the case after a re-start (aka catalog_xmin is NULL).
>>>>
>>>> I think a re-start and reload should produce identical behavior on
>>>> the primary physical slot. If so, I'm tempted to think that the catalog_xmin
>>>> should be updated in case of a re-start too (even if all the logical slots are invalidated)
>>>> because the slots are not dropped yet. What do you think?
>>>
>>> I can't quite follow the steps leading up to the difference. Could you list
>>> them in a bit more detail?
>>>
>>>
>>
>> Sure, so with:
>>
>> 1) hot_standby_feedback set to off on the standby
>> 2) create 2 logical replication slots on the standby and activate one
>> 3) Invalidate the logical slots on the standby with VACUUM FULL on the primary
>> 4) change hot_standby_feedback to on on the standby
>>
>> If:
>>
>> 5) pg_reload_conf() on the standby, then on the primary we get a catalog_xmin
>> for the physical slot that the standby is attached to:
>>
>> postgres=# select slot_type,xmin,catalog_xmin  from pg_replication_slots ;
>>   slot_type | xmin | catalog_xmin
>> -----------+------+--------------
>>   physical  |  822 |          748
>> (1 row)
> 
> How long did you wait for this to change? 

Almost instantaneous after pg_reload_conf() on the standby.

> I don't think there's anything right
> now that'd force a new hot-standby-feedback message to be sent to the primary,
> after slots got invalidated.
> 
> I suspect that if you terminated the walsender connection on the primary,
> you'd not see it anymore either?
> 

Still there after the standby is shutdown but disappears when the standby is re-started.

> If that isn't it, something is broken in InvalidateObsolete...
> 

Will look at what's going on and ensure catalog_xmin is not sent to the primary after pg_reload_conf() (if the slots are invalidated).

> 
>> No, but a question still remains to me:
>>
>> Given the fact that the row removal case is already done
>> in the next test (aka Scenario 2), If we want to replace the "vacuum full" test
>> on the database (done in Scenario 1) with a cheaper one at the table level,
>> what could it be to guarantee an invalidation?
>>
>> Same as scenario 2 but with "vacuum full pg_class" would not really add value
>> to the tests, right?
> 
> A database wide VACUUM FULL is also just a row removal test, no? 

Yeah, so I was wondering if Scenario 1 was simply not just useless.

> I think it
> makes sense to test that both VACUUM and VACUUM FULL both trigger conflicts,
> because they internally use *very* different mechanisms.  

Got it, will do and replace Scenario 1 as you suggested initially.

> It'd probably be
> good to test at least conflicts triggered due to row removal via on-access
> pruning as well. And perhaps also for btree killtuples.  I think those are the
> common cases for catalog tables.
> 

Thanks for the proposal, will look at it.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com