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)
