Re: [PATCH] vacuum: stop using stream ring after failsafe

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Jingtang Zhang <mrdrivingduck(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: [PATCH] vacuum: stop using stream ring after failsafe
Date: 2026-08-06 20:42:38
Message-ID: CAAKRu_YLE-DS+K=jB5fsBE6u3u+a3nMdFLG=_+c_TSjLjL80mw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 3, 2026 at 6:45 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> For master, I actually think what we should do is save the IOContext
> in the ReadBuffersOperation instead of the BufferAccessStrategy. I
> think it is cleaner since you only need the IOContext when completing
> the IO and you can pass the BufferAccessStrategy directly to
> StartReadBuffers() without saving it in the ReadBuffersOperation.
>
> And, when it comes to this patch and being able to "deactivate" the
> buffer access strategy, we need a way to do so for future IOs without
> affecting in-progress IOs, and this structure seems like the best way
> to do that.

Andres pointed out off-list that we had seen regressions when adding
another parameter to StartReadBuffersImpl() because it currently has 6
arguments and the sysv-x86-64 ABI can only accommodate 6 arguments in
registers before spilling to stack. As such, most of my "elegant"
solutions don't work. One thing we could do is add another
StartReadBuffersImpl() flag for "use strategy". I tried that and
didn't love how it looked since we then have to check that flag before
using the strategy in 5 places and that can only grow in the future.

Instead, I propose we just take a small hit to the accounting and
simply clear the ReadBuffersOperations->strategy even of in-progress
IOs. It should only misattribute a bit of wait time -- and only for a
limited number of IOs once -- when we enter failsafe mode. It makes
for a simple, backpatchable solution. I've attached here what I intend
to commit later today.

- Melanie

Attachment Content-Type Size
0001-Restore-vacuum-failsafe-abandonment-of-buffer-access.patch text/x-patch 6.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2026-08-06 21:38:28 Re: problems with toast.* reloptions
Previous Message Corey Huinker 2026-08-06 20:17:10 Re: Credits For v19