From: Antonin Houska <ah@cybertec.at>
To: Robert Haas <robertmhaas@gmail.com>
cc: Stephen Frost <sfrost@snowman.net>,
    PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Temporary file access API
In-reply-to: <CA+TgmobnojZw6mZi0T_61RtUDgsxroYhp+DJU5Yhz6caxUrQgw@mail.gmail.com>
References: <4987.1644323098@antos> <20220301143431.GT10577@tamriel.snowman.net> <27013.1646738099@antos> <CA+TgmobnojZw6mZi0T_61RtUDgsxroYhp+DJU5Yhz6caxUrQgw@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
   message dated "Tue, 05 Apr 2022 15:29:38 -0400."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3145.1649408068.1@antos>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 08 Apr 2022 10:54:28 +0200
Message-ID: <3146.1649408068@antos>
Archived-At: <https://www.postgresql.org/message-id/3146.1649408068%40antos>
Precedence: bulk

Robert Haas <robertmhaas@gmail.com> wrote:

> On Tue, Mar 8, 2022 at 6:12 AM Antonin Houska <ah@cybertec.at> wrote:
> > Thanks for your comments, the initial version is attached here.
> =

> I've been meaning to look at this thread for some time but have not
> found enough time to do that until just now. And now I have
> questions...
> =

> 1. Supposing we accepted this, how widely do you think that we could
> adopt it? I see that this patch set adapts a few places to use it and
> that's nice, but I have a feeling there's a lot more places that are
> making use of system calls directly, or through some other
> abstraction, than just this. I'm not sure that we actually want to use
> something like this everywhere, but what would be the rule for
> deciding where to use it and where not to use
> it? If the plan for this facility is just to adapt these two
> particular places to use it, that doesn't feel like enough to be
> worthwhile.

Admittedly I viewed the problem from the perspective of the TDE, so I have=
n't
spent much time looking for other opportunities. Now, with the stats colle=
ctor
using shared memory, even one of the use cases implemented here no longer
exists. I need to do more research.

Do you think that the use of a system call is a problem itself (e.g. becau=
se
the code looks less simple if read/write is used somewhere and fread/fwrit=
e
elsewhere; of course of read/write is mandatory in special cases like WAL,
heap pages, etc.)  or is the problem that the system calls are used too
frequently? I suppose only the latter.

Anyway, I'm not sure there are *many* places where system calls are used t=
oo
frequently. Instead, the coding uses to be such that the information is fi=
rst
assembled in memory and then written to file at once. So the value of the
(buffered) stream is that it makes the code simpler (eliminates the need t=
o
prepare the data in memory). That's what I tried to do for reorderbuffer.c=
 and
pgstat.c in my patch.

Related question is whether we should try to replace some uses of the libc
stream (FILE *) at some places. You seem to suggest that in [1]. One examp=
le
is snapmgr.c:ExportSnapshot(), if we also implement output formatting. Of
course there are places where (FILE *) cannot be replaced because, besides
regular file, the code needs to work with stdin/stdout in general. (Parsin=
g of
configuration files falls into this category, but that doesn't matter beca=
use
bison-generated parser seems to implement buffering anyway.)

> 2. What about frontend code? Most frontend code won't examine data
> files directly, but at least pg_controldata, pg_checksums, and
> pg_resetwal are exceptions.

If the frequency of using system calls is the problem, then I wouldn't cha=
nge
these because ControlFileData structure needs to be initialized in memory
anyway and then written at once. And pg_checksums reads whole blocks
anyway. I'll take a closer look.

> 3. How would we extend this to support encryption? Add an extra
> argument to BufFileStreamInit(V)FD passing down the encryption
> details?

Yes.

> There are some smaller things about the patch with which I'm not 100%
> comfortable, but I'd like to start by understanding the big picture.

Thanks!

[1] https://www.postgresql.org/message-id/CA+TgmoYGjN_f=3DFCErX49bzjhNG+Go=
ctY+a+XhNRWCVvDY8U74w@mail.gmail.com

-- =

Antonin Houska
Web: https://www.cybertec-postgresql.com