Message-ID: <5bd4cbfe-fc71-c576-851d-3bb21ef5c080@gmail.com>
Date: Tue, 25 Jul 2023 13:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #18014: Releasing catcache entries makes
 schema_to_xmlschema() fail when parallel workers are used
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-bugs@lists.postgresql.org
References: <b2dcd258-82f1-dfe3-498e-9f652a0c0c4b@gmail.com>
 <20230720.162802.905550494964943231.horikyota.ntt@gmail.com>
 <20230720.164459.2183367718473165651.horikyota.ntt@gmail.com>
 <20230720.172924.535889842187000361.horikyota.ntt@gmail.com>
 <1a2d5aba-df29-ab8d-65ac-6ffd54d1ac62@gmail.com>
 <969017.1689960055@sss.pgh.pa.us>
 <00e24aa6-5d03-99b6-ab41-495a35fc93f6@gmail.com>
 <982076.1689967306@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <982076.1689967306@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/5bd4cbfe-fc71-c576-851d-3bb21ef5c080%40gmail.com>
Precedence: bulk

Hi Tom,

21.07.2023 22:21, Tom Lane wrote:
> Yes, we certainly want to do that during LockRelationOid.  But what
> seems to be happening here is an inval while we are closing/unlocking
> the catalog we got the syscache entry from.  That is, the expected
> behavior here is:
>
> SearchSysCacheExists:
>
>    * is entry present-and-valid?
>      No, so...
>
>    * open and lock relevant catalog (with possible inval)
>
>    * scan catalog, find desired row, create valid syscache entry
>
>    * close and unlock catalog
>
>    * return success
>
> SearchSysCache1 (from pg_class_aclmask_ext):
>
>    * is entry present-and-valid?
>      Yes, so increment its refcount and return it
>
> There is no inval in the entry-already-present code path in syscache
> lookup.  So if we are seeing this failure, ISTM it must mean that an
> inval is happening during "close and unlock catalog", which seems like
> something that we don't want.  But I've not traced exactly how that
> happens.

Yes, but here we deal with -DCATCACHE_FORCE_RELEASE (added to config_env
on prion), so the cache entry, that was just found in
SearchSysCacheExists(), is removed immediately because of
SearchSysCacheExists() ->  ReleaseSysCache(tuple) -> ReleaseCatCache(tuple).

So, while the construction "if (SearchSysCacheExists()) ... SearchSysCache1()"
seems robust for normal conditions, it might be broken when catcache entries
released forcefully. Thus, if the worst consequence of the issue is sporadic
test failures on prion, then may be fix it in a least invasive way (on level 1).

Best regards,
Alexander