Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Karina Litskevich <litskevichkarina(at)gmail(dot)com>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Cc: Gurjeet Singh <gurjeet(at)singh(dot)im>, Richard Guo <guofenglinux(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Avoid unncessary always true test (src/backend/storage/buffer/bufmgr.c)
Date: 2023-09-04 12:32:56
Message-ID: e96ba136-282d-995f-d956-453a029cf5d3@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 30.06.23 20:48, Karina Litskevich wrote:
> So as for me calling LockRelationForExtension() and
> UnlockRelationForExtension()
> without testing eb.rel first looks more like a bug here. However, they
> are never
> actually called with eb.rel=NULL because of the EB_* flags, so there is
> no bug
> here. I believe we should add Assert checking that when eb.rel is NULL,
> flags
> are such that we won't use eb.rel. And yes, we can remove unnecessary checks
> where the flags value guaranty us that eb.rel is not NULL.
>
> And another minor observation. It seems to me that we don't need a
> "could have
> been closed while waiting for lock" in ExtendBufferedRelShared(), because I
> believe the comment above already explains why updating eb.smgr:
>
>  * Note that another backend might have extended the relation by the time
>  * we get the lock.
>
> I attached the new version of the patch as I see it.

This patch version looks the most sensible to me. But as commented
further downthread, some explanation around the added assertions would
be good.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2023-09-04 12:34:28 Re: [17] CREATE SUBSCRIPTION ... SERVER
Previous Message Ashutosh Bapat 2023-09-04 12:31:57 Re: [17] CREATE SUBSCRIPTION ... SERVER