Re: Temporary file access API

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.

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.

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.