Re: BufferAlloc: don't take two simultaneous locks

From: Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, y(dot)sokolov(at)postgrespro(dot)ru, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: BufferAlloc: don't take two simultaneous locks
Date: 2022-02-06 16:34:54
Message-ID: CANtu0ogqNyceN0WCS186OnBLhw71xzwGDUw+Yk3Qt4J9TcM_mg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, Yura.

A one additional moment:

> 1332: Assert((oldFlags & (BM_PIN_COUNT_WAITER | BM_IO_IN_PROGRESS)) == 0);
> 1333: CLEAR_BUFFERTAG(buf->tag);
> 1334: buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK);
> 1335: UnlockBufHdr(buf, buf_state);

I think there is no sense to unlock buffer here because it will be
locked after a few moments (and no one is able to find it somehow). Of
course, it should be unlocked in case of collision.

BTW, I still think is better to introduce some kind of
hash_update_hash_key and use it.

It may look like this:

// should be called with oldPartitionLock acquired
// newPartitionLock hold on return
// oldPartitionLock and newPartitionLock are not taken at the same time
// if newKeyPtr is present - existingEntry is removed
bool hash_update_hash_key_or_remove(
HTAB *hashp,
void *existingEntry,
const void *newKeyPtr,
uint32 newHashValue,
LWLock *oldPartitionLock,
LWLock *newPartitionLock
);

Thanks,
Michail.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2022-02-06 16:38:26 Re: [PATCH v2] use has_privs_for_role for predefined roles
Previous Message Robert Haas 2022-02-06 15:45:25 Re: pg_walinspect - a new extension to get raw WAL data and WAL stats