On Fri, Feb 7, 2020 at 1:37 PM Andres Freund <and...@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
0001-Add-a-wrapper-for-fstat-for-when-you-just-want-the-s.patch
Description: Binary data
0002-Use-pg_pread-and-pg_pwrite-in-slru.c.patch
Description: Binary data
0003-Use-pg_fstat_size-in-walsender.c.patch
Description: Binary data
