Message-ID: <ea198f85-5df0-464d-8a0a-45bf88166c33@iki.fi>
Date: Fri, 13 Dec 2024 16:20:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Recovering from detoast-related catcache invalidations
To: Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Xiaoran Wang <fanfuxiaoran@gmail.com>,
 pgsql-hackers@lists.postgresql.org, Andres Freund <andres@anarazel.de>
References: 
 <CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com>
 <329632.1705011663@sss.pgh.pa.us>
 <CAGjhLkOFmzx9gbB8qudKxdg8UR0SApo-thKuPZ=1zJaZ6-ORXQ@mail.gmail.com>
 <754826.1705090452@sss.pgh.pa.us> <793469.1705092433@sss.pgh.pa.us>
 <20240114201411.d0@rfd.leadboat.com> <20240924212036.0a.nmisch@google.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <20240924212036.0a.nmisch@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/ea198f85-5df0-464d-8a0a-45bf88166c33%40iki.fi>
Precedence: bulk

On 25/09/2024 00:20, Noah Misch wrote:
> On Sun, Jan 14, 2024 at 12:14:11PM -0800, Noah Misch wrote:
>> On Fri, Jan 12, 2024 at 03:47:13PM -0500, Tom Lane wrote:
>>> Oh!  After nosing around a bit more I remembered systable_recheck_tuple,
>>> which is meant for exactly this purpose.  So v4 attached.
>>
>> systable_recheck_tuple() is blind to heap_inplace_update(), so it's not a
>> general proxy for invalidation messages.  The commit for $SUBJECT (ad98fb1)
>> doesn't create any new malfunctions, but I expect the systable_recheck_tuple()
>> part will change again before the heap_inplace_update() story is over
>> (https://postgr.es/m/flat/CAMp+ueZQz3yDk7qg42hk6-9gxniYbp-=bG2mgqecErqR5gGGOA@mail.gmail.com).
> 
> Commit f9f47f0 (2024-06-27) addressed inplace updates here.

I started to wonder if there's still an issue with catcache list 
entries. The code to build a CatCList looks like this:

SearchCatCacheList()
   systable_beginscan()
   while (HeapTupleIsValid(ntp = systable_getnext(scandesc))) {
     ct = CatalogCacheCreateEntry(ntp)
     if (ct == NULL)
     {
       /* 'ntp' was concurrently invalidated, start all over */
     }
   }
   systable_endscan();

   /* create CatCList entry */


CatalogCacheCreateEntry() can accept catcache invalidations when it 
opens the toast table, and it now has recheck logic to detect the case 
that the tuple it's processing (ntp) is invalidated. However, isn't it 
also possible that it accepts an invalidation message for a tuple that 
we had processed in an earlier iteration of the loop? Or that a new 
catalog tuple was inserted that should be part of the list we're building?

-- 
Heikki Linnakangas
Neon (https://neon.tech)