Re: Getting rid of some more lseek() calls - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: Getting rid of some more lseek() calls
Date
Msg-id CA+hUKGJ1FJitVDyLEngQ8yrvUoFa_+8V=063dBU9SR4ZjTp6Bw@mail.gmail.com
Whole thread Raw
In response to Re: Getting rid of some more lseek() calls  (Andres Freund <andres@anarazel.de>)
Responses Re: Getting rid of some more lseek() calls
List pgsql-hackers
On Fri, Feb 7, 2020 at 1:37 PM Andres Freund <andres@anarazel.de> wrote:
> Hm. This still leaves us with one source of SLRU_SEEK_FAILED. And that's
> really just for getting the file size. Should we rename that?
>
> Perhaps we should just replace lseek(SEEK_END) with fstat()? Or at least
> one wrapper function for getting the size? It seems ugly to change fd
> positions just for the purpose of getting file sizes (and also implies
> more kernel level locking, I believe).

lseek(SEEK_END) seems to be nearly twice as fast as fstat() if you
just call it in a big loop, on Linux and FreeBSD (though I didn't
investigate exactly why, mitigations etc, it certainly returns more
stuff so there's that).  I don't think that's a problem here (I mean,
we open and close the file every time so we can't be too concerned
about the overheads), so I'm in favour of creating a pg_fstat_size(fd)
function on aesthetic grounds.  Here's a patch like that; better names
welcome.

For the main offender, namely md.c via fd.c's FileSize(), I'd hold off
on changing that until we figure out how to cache the sizes[1].

> There's still a few more lseek(SEEK_SET) calls in the backend after this
> (walsender, miscinit, pg_stat_statements). It'd imo make sense to just
> try to get rid of all of them in one series this time round?

Ok, I pushed changes for all the cases discussed except slru.c and
walsender.c, which depend on the bikeshed colour discussion about
whether we want pg_fstat_size().  See attached.

[1]
https://www.postgresql.org/message-id/flat/CAEepm%3D3SSw-Ty1DFcK%3D1rU-K6GSzYzfdD4d%2BZwapdN7dTa6%3DnQ%40mail.gmail.com

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Improve heavyweight locks instead of building new lock managers?
Next
From: Adam Lee
Date:
Subject: [PATCH] Sanity check BackgroundWorker's function entry