Date: Wed, 28 Jul 2021 11:26:42 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: "Drouvot, Bertrand" <bdrouvot@amazon.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>
Subject: Re: Minimal logical decoding on standbys
Message-ID: <202107281526.o74ieuj6sj7z@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <fa90206a-6bf1-2121-cf0c-15b0734a2731@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202107281526.o74ieuj6sj7z%40alvherre.pgsql>
Precedence: bulk

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.
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.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Investigación es lo que hago cuando no sé lo que estoy haciendo"
(Wernher von Braun)