public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: [email protected]
To: Tomas Vondra <[email protected]>
To: Anastasia Lubennikova <[email protected]>
To: Pavan Deolasee <[email protected]>
To: Jeff Janes <[email protected]>
Subject: hio.c does visibilitymap_pin()/IO while holding buffer lock
Date: Fri, 24 Mar 2023 19:57:40 -0700
Message-ID: <[email protected]> (raw)

Hi,

Starting with

commit 7db0cd2145f2bce84cac92402e205e4d2b045bf2
Author: Tomas Vondra <[email protected]>
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

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: hio.c does visibilitymap_pin()/IO while holding buffer lock
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox