Re: Streaming I/O, vectored I/O (WIP)
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Robert Haas <robertmhaas@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>,
Andres Freund <andres@anarazel.de>, Melanie Plageman <melanieplageman@gmail.com>
Date: 2024-04-02T08:39:49Z
Lists: pgsql-hackers
Attachments
- v15-0001-Provide-vectored-variant-of-ReadBuffer.patch (text/x-patch) patch v15-0001
- v15-0002-Provide-API-for-streaming-relation-data.patch (text/x-patch) patch v15-0002
- v15-0003-Use-streaming-I-O-in-pg_prewarm.patch (text/x-patch) patch v15-0003
I had been planning to commit v14 this morning but got cold feet with the BMR-based interface. Heikki didn't like it much, and in the end, neither did I. I have now removed it, and it seems much better. No other significant changes, just parameter types and inlining details. For example: * read_stream_begin_relation() now takes a Relation, likes its name says * StartReadBuffers()'s operation takes smgr and optional rel * ReadBuffer_common() takes smgr and optional rel ReadBuffer() (which calls ReadBuffer_common() which calls StartReadBuffer() as before) now shows no regression in a tight loop over ~1 million already-in-cache pages (something Heikki had observed before and could only completely fix with a change that affected all callers). The same test using read_stream.c is still slightly slower, ~1 million pages -in-cache pages 301ms -> 308ms, which seems acceptable to me and could perhaps be chased down with more study of inlining/specialisation. As mentioned before, it doesn't seem to be measurable once you actually do something with the pages. In some ways BMR was better than the "fake RelationData" concept (another attempt at wrestling with the relation vs storage duality, that is, the online vs recovery duality). But in other ways it was worse: a weird inconsistent mixture of pass-by-pointer and pass-by-value interfaces that required several code paths to handle it being only partially initialised, which turned out to be wasted cycles implicated in regressions, despite which it is not even very nice to use anyway. I'm sure it could be made to work better, but I'm not yet sure it's really needed. In later work for recovery I will need to add a separate constructor read_stream_begin_smgr_something() anyway for other reasons (multi-relation streaming, different callback) and perhaps also a separate StartReadBuffersSmgr() if it saves measurable cycles to strip out branches. Maybe it was all just premature pessimisation. So this is the version I'm going to commit shortly, barring objections.
Commits
-
Fix typos and incorrect type in read_stream.c
- 2ea4b2927722 17.0 landed
-
Use streaming I/O in pg_prewarm.
- 3a352df05e65 17.0 landed
-
Provide API for streaming relation data.
- b5a9b18cd0bc 17.0 landed
-
Provide vectored variant of ReadBuffer().
- 210622c60e1a 17.0 landed
-
Provide vectored variants of smgrread() and smgrwrite().
- 4908c5872059 17.0 landed
-
Provide multi-block smgrprefetch().
- b485ad7f07c8 17.0 landed
-
Provide vectored variants of FileRead() and FileWrite().
- 871fe4917e1e 17.0 landed
-
Provide helper for retrying partial vectored I/O.
- 0c6be59f5e34 17.0 landed
-
Optimize pg_readv/pg_pwritev single vector case.
- 15c9ac362993 17.0 landed
-
bufmgr: Support multiple in-progress IOs by using resowner
- 12f3867f5534 16.0 cited