From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | jian he <jian(dot)universality(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Lakhin <exclusion(at)gmail(dot)com> |
Subject: | Re: Relcache refactoring |
Date: | 2024-10-31 08:14:19 |
Message-ID: | ddb86292-49be-4ec8-b38e-918fb24c2cec@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 23/09/2024 04:39, jian he wrote:
> static void
> RelationClearRelation(Relation relation)
> {
> Assert(RelationHasReferenceCountZero(relation));
> Assert(!relation->rd_isnailed);
>
> /*
> * Relations created in the same transaction must never be removed, see
> * RelationFlushRelation.
> */
> Assert(relation->rd_createSubid == InvalidSubTransactionId);
> Assert(relation->rd_firstRelfilelocatorSubid == InvalidSubTransactionId);
> Assert(relation->rd_droppedSubid == InvalidSubTransactionId);
>
> /* Ensure it's closed at smgr level */
> RelationCloseSmgr(relation);
>
> /* Free AM cached data, if any */
> if (relation->rd_amcache)
> pfree(relation->rd_amcache);
> relation->rd_amcache = NULL;
>
> /* Mark it as invalid (just pro forma since we will free it below) */
> relation->rd_isvalid = false;
>
> /* Remove it from the hash table */
> RelationCacheDelete(relation);
>
> /* And release storage */
> RelationDestroyRelation(relation, false);
> }
>
>
> can be simplified as
>
>
> static void
> RelationClearRelation(Relation relation)
> {
> ---bunch of Asserts
>
> /* first mark it as invalid */
> RelationInvalidateRelation(relation);
>
> /* Remove it from the hash table */
> RelationCacheDelete(relation);
>
> /* And release storage */
> RelationDestroyRelation(relation, false);
> }
> ?
>
>
> in RelationRebuildRelation
> we can also use RelationInvalidateRelation?
Yep, that makes sense, changed them that way.
> * We assume that at the time we are called, we have at least AccessShareLock
> * on the target index. (Note: in the calls from RelationClearRelation,
> * this is legitimate because we know the rel has positive refcount.)
>
> calling RelationClearRelation, rel->rd_refcnt == 0
> seems conflicted with the above comments in RelationReloadIndexInfo.
> so i am confused with the above comments.
That comment is outdated, because after these patches, there's only once
caller, in RelationRebuildRelation. Fixed that, thanks for noticing!
Hmm, in case of a nailed index, this will get called with refcnt == 1,
even though we have no AccessShareLock on the index. But I guess that's
OK for a nailed index.
Committed with those fixes, thanks for the review!
--
Heikki Linnakangas
Neon (https://neon.tech)
From | Date | Subject | |
---|---|---|---|
Next Message | Anthonin Bonnefoy | 2024-10-31 08:23:47 | Re: Segfault in jit tuple deforming on arm64 due to LLVM issue |
Previous Message | Soumyadeep Chakraborty | 2024-10-31 08:03:06 | Re: InstallXLogFileSegment() vs concurrent WAL flush |