Re: Relcache refactoring

On 31/10/2024 12:06, Heikki Linnakangas wrote:
> On 31/10/2024 10:14, Heikki Linnakangas wrote:
>> Committed with those fixes, thanks for the review!
> 
> There is a failure in the buildfarm animal 'prion', which builds with 
> -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE options. I'll look 
> into that later today. (I did test with -DRELCACHE_FORCE_RELEASE on my 
> laptop, but must've missed something).

I was finally able to reproduce this, by running the failing pg_regress 
tests concurrently with repeated "vacuum full pg_database" calls. It's 
curious that 'prion' failed on the first run after the commit, I was not 
able to reproduce it by just running the unmodified regression tests 
with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE.

Committed a fix. There was one codepath that was missing a call to 
RelationInitPhysicalAddr(relation) after the patch. I alluded to this 
earlier in this thread:

> ## RelationInitPhysicalAddr() call in RelationReloadNailed()
> 
> One question or little doubt I have: Before these patches, 
> RelationReloadNailed() calls RelationInitPhysicalAddr() even when it 
> leaves the relation as invalidated because we're not in a transaction or 
> if the relation isn't currently in use. That's a bit surprising, I'd 
> expect it to be done when the entry is reloaded, not when it's 
> invalidated. That's how it's done for non-nailed relations. And in fact, 
> for a nailed relation, RelationInitPhysicalAddr() is called *again* when 
> it's reloaded.
> 
> Is it important? Before commit a54e1f1587, nailed non-index relations 
> were not reloaded at all, except for the call to 
> RelationInitPhysicalAddr(), which seemed consistent. I think this was 
> unintentional in commit a54e1f1587, or perhaps just overly defensive, as 
> there's no harm in some extra RelationInitPhysicalAddr() calls.
> 
> This patch removes that extra call from the invalidation path, but if it 
> turns out to be wrong, we can easily add it to RelationInvalidateRelation.

Now this wasn't exactly that, but related.

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