Re: Minimal logical decoding on standbys

Hi,

On 9/9/21 9:17 AM, Drouvot, Bertrand wrote:
>
> Hi Alvaro,
>
> On 8/2/21 4:56 PM, Drouvot, Bertrand wrote:
>> Hi Alvaro,
>>
>> On 7/28/21 5:26 PM, Alvaro Herrera wrote:
>>> On 2021-Jul-27, Drouvot, Bertrand wrote:
>>>
>>>> diff --git a/src/backend/utils/cache/lsyscache.c
>>>> b/src/backend/utils/cache/lsyscache.c
>>>> +bool
>>>> +get_rel_logical_catalog(Oid relid)
>>>> +{
>>>> +     bool    res;
>>>> +     Relation rel;
>>>> +
>>>> +     /* assume previously locked */
>>>> +     rel = table_open(relid, NoLock);
>>>> +     res = RelationIsAccessibleInLogicalDecoding(rel);
>>>> +     table_close(rel, NoLock);
>>>> +
>>>> +     return res;
>>>> +}
>>> So RelationIsAccessibleInLogicalDecoding() does a cheap check for
>>> wal_level which can be done without opening the table; I think this
>>> function should be rearranged to avoid doing that when not needed.
>>
>> Thanks for looking at it.
>>
>>
>>> Also, putting this function in lsyscache.c seems somewhat wrong since
>>> it's not merely accessing the system caches ...
>>>
>>> I think it would be better to move this elsewhere (relcache.c, proto in
>>> relcache.h, perhaps call it RelationIdIsAccessibleInLogicalDecoding)
>>> and
>>> short-circuit for the check that can be done before opening the table.
>
> So you have in mind to check for XLogLogicalInfoActive() first, and if
> true, then open the relation and call
> RelationIsAccessibleInLogicalDecoding()?
>
> If so, then what about also creating a new
> RelationIsAccessibleWhileLogicalWalLevel() or something like this
> doing the same as RelationIsAccessibleInLogicalDecoding() but without
> the XLogLogicalInfoActive() check?
>
>>> At least the GiST code appears to be able to call this several times
>>> per
>>> vacuum run, so it makes sense to short-circuit it for the fast case.
>>>
>>> ... though looking at the GiST code again I wonder if it would be more
>>> sensible to just stash the table's Relation pointer somewhere in the
>>> context structs
>
> Do you have a "good place" in mind?
>
Another rebase attached.

The patch proposal to address Andre's walsender corner cases is still a
dedicated commit (as i think it may be easier to discuss).

Thanks

Bertrand