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