Re: Relcache refactoring

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Alexander Lakhin <exclusion(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>
Subject: Re: Relcache refactoring
Date: 2024-10-31 16:29:13
Message-ID: c638873f-8b1e-4770-ba49-5a0b3e140cd9@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2024-10-31 16:29:56 [PATCH] Improve code coverage of network address functions
Previous Message Jacob Champion 2024-10-31 16:24:11 Re: [PoC] Federated Authn/z with OAUTHBEARER