Antonin Houska <a...@cybertec.at> wrote: > Peter Eisentraut <peter.eisentr...@enterprisedb.com> wrote: > > > On 04.07.22 18:35, Antonin Houska wrote: > > >> Attached is a new version of the patch, to evaluate what the API use in > > >> the > > >> backend could look like. I haven't touched places where the file is > > >> accessed > > >> in a non-trivial way, e.g. lseek() / fseek() or pg_pwrite() / pg_pread() > > >> is > > >> called. > > > Rebased patch set is attached here, which applies to the current master. > > > (A few more opportunities for the new API implemented here.) > > > > I don't understand what this patch set is supposed to do. AFAICT, the > > thread > > originally forked off from a TDE discussion and, considering the thread > > subject, was possibly once an attempt to refactor temporary file access to > > make integrating encryption easier? The discussion then talked about things > > like saving on system calls and excising all OS-level file access API use, > > which I found confusing, and the thread then just became a general > > TDE-related > > mini-discussion. > > Yes, it's an attempt to make the encryption less invasive, but there are a few > other objectives, at least: 1) to make the read / write operations "less > low-level" (there are common things like error handling which are often just > copy & pasted from other places), 2) to have buffered I/O with configurable > buffer size (the current patch version still has fixed buffer size though) > > It's true that the discussion ends up to be encryption-specific, however the > scope of the patch is broader. The first meassage of the thread references a > related discussion > > https://www.postgresql.org/message-id/CA+TgmoYGjN_f=fcerx49bzjhng+gocty+a+xhnrwcvvdy8u...@mail.gmail.com > > which is more important for this patch than the suggestions about encryption. > > > The patches at hand extend some internal file access APIs and then sprinkle > > them around various places in the backend code, but there is no explanation > > why or how this is better. I don't see any real structural savings one might > > expect from a refactoring patch. No information has been presented how this > > might help encryption. > > At this stage I expected feedback from the developers who have already > contributed to the discussion, because I'm not sure myself if this version > fits their ideas - that's why I didn't elaborate the documentation yet. I'll > try to summarize my understanding in the next version, but I'd appreciate if I > got some feedback for the current version first. > > > I also suspect that changing around the use of various file access APIs > > needs > > to be carefully evaluated in each individual case. Various code has subtle > > expectations about atomic write behavior, syncing, flushing, error recovery, > > etc. I don't know if this has been considered here. > > I considered that, but could have messed up at some places. Right now I'm > aware of one problem: pgstat.c does not expect the file access API to raise > ERROR - this needs to be fixed.
Attached is a new version. It allows the user to set "elevel" (i.e. ERROR is not necessarily thrown on I/O failure, if the user prefers to check the number of bytes read/written) and to specify the buffer size. Also, 0015 adds a function to copy data from one file descriptor to another. -- Antonin Houska Web: https://www.cybertec-postgresql.com
temp_file_api_v4.tgz
Description: application/gzip
