Subject: Re: Minimal logical decoding on standbys
To: Alvaro Herrera <alvherre@2ndquadrant.com>
CC: Ibrar Ahmed <ibrar.ahmad@gmail.com>, Amit Khandekar
	<amitdkhan.pg@gmail.com>, Andres Freund <andres@anarazel.de>,
	<fabriziomello@gmail.com>, tushar <tushar.ahuja@enterprisedb.com>, "[pgdg]
 Robert Haas" <robertmhaas@gmail.com>, Rahila Syed
	<rahila.syed@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
References: <202107281526.o74ieuj6sj7z@alvherre.pgsql>
From: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Message-ID: <ae1f2199-65e7-a431-d79e-c32b9379aa6e@amazon.com>
Date: Mon, 2 Aug 2021 16:56:44 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.12.0
MIME-Version: 1.0
In-Reply-To: <202107281526.o74ieuj6sj7z@alvherre.pgsql>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Archived-At: 
 <https://www.postgresql.org/message-id/ae1f2199-65e7-a431-d79e-c32b9379aa6e%40amazon.com>
Precedence: bulk

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.
> 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 instead of opening and closing it time and again.

That does make sense, I'll look at it.

Bertrand