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)