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)
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 |