Feedback-ID: id4a34324:Fastmail
Date: Thu, 8 Jan 2026 19:29:35 -0500
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.com>, 
	Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, 
	Thomas Munro <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>, 
	Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Message-ID: <4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf>
Reply-To: Andres Freund <andres@anarazel.de>
References: <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
 <CAAKRu_ZcZCXNUWfhEg4qOYuRO8T1x3_gvaUFFrwgCfY0RnVhkg@mail.gmail.com>
 <3nce7i72ayzkunai6mkz24ckbxk74jodz4ua2chcdrwppxlxcd@w6x5kfkjrkru>
 <lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg@noioxkquezdw>
 <c00d4a9f-c0f8-49ca-bf27-29c6641f2fc6@iki.fi>
 <q3ybyaiownp4ivt6xfy55s5llvtq6vqsd4zbt5kn6hacxdcpp4@2ngd4h4bev5d>
 <dsbacri4xldlobl3jbqqlksefsyrwj7lzljbohxsbqjus3z4o7@otb6rge4w7az>
 <1108f18d-cf7c-4f17-b29c-a119fe42f7e5@iki.fi>
 <5dwlfu2jyzkyf3nrlzxxblxctb6xio5es73ptgsahjnmfu5miu@772rc764hfhi>
 <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="rkiyqpij3ajqn7ww"
Content-Disposition: inline
In-Reply-To: <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
From: Andres Freund <andres@anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5%40usw7feyb5bzf>
Precedence: bulk


--rkiyqpij3ajqn7ww
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

I pushed what was 0001, 0002 in v8. Attached is an updated set of patches for
the rest.

Main changes:

- the explanation in "heapam: Use exclusive lock on old page in CLUSTER" as to
  why it's problematic to not set hint bits wasn't quite right. I've updated
  it:

    heapam: Use exclusive lock on old page in CLUSTER

    To be able to guarantee that we can set the hint bit, acquire an exclusive
    lock on the old buffer. This is required as a future commit will only allow
    hint bits to be set with a new lock level, which is acquired as-needed in a
    non-blocking fashion.

    We need the hint bits, set in heapam_relation_copy_for_cluster() ->
    HeapTupleSatisfiesVacuum(), to be set, as otherwise reform_and_rewrite_tuple()
    -> rewrite_heap_tuple() will get confused. Specifically, rewrite_heap_tuple()
    checks for HEAP_XMAX_INVALID in the old tuple to determine whether to check
    the old-to-new mapping hash table.

- I added a patch that inverts the meaning of LW_FLAG_RELEASE_OK, to make the
  equivalent code for content locks easier. For buffer content locks we reset
  the flags when invalidating, and otherwise we'd either need to not have the
  equivalent of LW_FLAG_RELEASE_OK in the flag mask or explicitly add it after
  making the buffer valid.

  I think it's also nicer this way round, because we e.g. can assert that
  there are no pending wakeups when invalidating a buffer.

- I added a patch to reorganize some of the flags stuff in buf_internals.h, to
  make the later patches cleaner. In particular flags are now defined with a
  macro so that changing at which offset flag bits are doesn't require
  touching every single flag value.

- For the main commit, the reorganized flag stuff removed one of the remaining
  FIXMEs.

- I removed the performance instrumentation stuff from the batch visibility
  commit.


I think 0001, 0002, 0003 can be committed. 0004, 0005 are new and probably
could use a sanity check.  0006 hasn't changed much and is imo pretty much
ready, but should be pushed together with 0007.  0007 is getting close, I
think.  0008-0010 need a bit more work, but I think that can wait until 0007
has been pushed.

For 0008, it'd be nice if somebody could look at the way buf_internals.h now
looks.

The only remaining FIXME in 0008 is about the the reuse of
PGPROC->{lwWaiting,lwWaitMode,lwWaitLink}. I think reusing them for content
locks isn't pretty, but it's probably not worth duplicating them. Thoughts?

Greetings,

Andres Freund

--rkiyqpij3ajqn7ww
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v9-0001-freespace-Don-t-modify-page-without-any-lock.patch"