| 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 |
| 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 |