Re: IPC/MultixactCreation on the Standby server

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, pgsql-hackers@lists.postgresql.org, Ivan Bykov <i.bykov@modernsys.ru>, Kirill Reshke <reshkekirill@gmail.com>
Date: 2025-11-28T19:51:41Z
Lists: pgsql-hackers

Attachments

On 28/11/2025 20:17, Andrey Borodin wrote:
> latest_page_number looks great! though I'm slightly worried by this commend:
> /*
>   * During XLOG replay, latest_page_number isn't necessarily set up
>   * yet; insert a suitable value to bypass the sanity test in
>   * SimpleLruTruncate.
>   */
> 
> and by
> 
> /*
>   * latest_page_number is the page number of the current end of the log;
>   * this is __not critical data__, since we use it only to avoid swapping out
>   * the latest page.
>   */
> 
> If we use it in startup process to prevent failure, now it is critical data.
> 
> But I think the comments are off, latest_page_number is updated precisely.

Agreed. I looked at the commit history of how the first comment got 
introduced, to see if it used to be true at some point, but AFAICS we've 
always kept latest_page_number up-to-date. It is very clearly 
initialized in StartupMultiXact(), before WAL replay. Furthermore, we 
only set the "suitable value to bypass the sanity test" for the offsets 
SLRU, but everywhere else where we truncate, extend or set 
latest_page_number, we treat the members SLRU. So I don't see how the 
offsets and members SLRUs could be different in that regard.

I think the second comment became outdated in commit bc7d37a525c0, which 
introduced the safety check in (what became later) SimpleLruTruncate(). 
After that, it's been important that latest_page_number is set 
correctly, although for the sanity check I guess you could be a little 
sloppy with it.

> The page initialization dance is only needed in back branches. And 
> we inevitable will have conflicts with SLRU refactoring in 18 and 
> banking in 17. Conceptually v12 looks good to me, I can prepare
> backport versions.

Thanks!

Here's a new patch version. I went through the test changes now:

I didn't understand why the 'kill9' and 'poll_start' stuff is needed. We 
have plenty of tests that kill the server with regular 
"$node->stop('immediate')", and restart the server normally. The 
checkpoint in the middle of the tests seems unnecessary too. I removed 
all that, and the test still seems to work. Was there a particular 
reason for them?

I moved the wraparound test to a separate test file and commit. More 
test coverage is good, but it's quite separate from the bugfix and the 
wraparound related test shares very little with the other test. The 
wraparound test needs a little more cleanup: use plain perl instead of 
'dd' and 'rm' for the file operations, for example. (I did that with the 
tests in the 64-bit mxoff patches, so we could copy from there.)

- Heikki

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix test to work with non-8kB block sizes

  2. Fix setting next multixid's offset at offset wraparound

  3. Add test for multixid wraparound

  4. Set next multixid's offset when creating a new multixid

  5. psql: Improve tab completion for large objects.

  6. Refactor some repetitive SLRU code

  7. Transaction IDs wrap around, per my proposal of 13-Aug-01. More