hio.c does visibilitymap_pin()/IO while holding buffer lock

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org, Tomas Vondra <tv(at)fuzzy(dot)cz>, Anastasia Lubennikova <lubennikovaav(at)gmail(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
Subject: hio.c does visibilitymap_pin()/IO while holding buffer lock
Date: 2023-03-25 02:57:40
Message-ID: 20230325025740.wzvchp2kromw4zqz@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Starting with

commit 7db0cd2145f2bce84cac92402e205e4d2b045bf2
Author: Tomas Vondra <tomas(dot)vondra(at)postgresql(dot)org>
Date: 2021-01-17 22:11:39 +0100

Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZE

RelationGetBufferForTuple does

/*
* The page is empty, pin vmbuffer to set all_frozen bit.
*/
if (options & HEAP_INSERT_FROZEN)
{
Assert(PageGetMaxOffsetNumber(BufferGetPage(buffer)) == 0);
visibilitymap_pin(relation, BufferGetBlockNumber(buffer), vmbuffer);
}

while holding a buffer lock. visibilitymap_pin() reads pages, if vmbuffer
doesn't already point to the right block.

The lock ordering rules are to lock VM pages *before* locking heap pages.

I think the reason this hasn't yet bitten us badly, is that INSERT_FROZEN
effectively requires that the relation is access exclusive locked. There
shouldn't be other backends locking multiple buffers in the relation (bgwriter
/ checkpointer can lock a single buffer at a time, but that's it).

I see roughly two ways forward:

1) We add a comment explaining why it's safe to violate lock ordering rules in
this one situation

2) Change relevant code so that we only return a valid vmbuffer if we could do
so without blocking / IO and, obviously, skip updating the VM if we
couldn't get the buffer.

Greetings,

Andres Freund

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-03-25 02:59:10 Re: [PoC] Let libpq reject unexpected authentication requests
Previous Message Michael Paquier 2023-03-25 02:56:22 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry