Re: Getting rid of some more lseek() calls

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Getting rid of some more lseek() calls
Date: 2020-02-11 05:04:09
Message-ID: CA+hUKGJ1FJitVDyLEngQ8yrvUoFa_+8V=063dBU9SR4ZjTp6Bw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Feb 7, 2020 at 1:37 PM Andres Freund <andres(at)anarazel(dot)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 Content-Type Size
0001-Add-a-wrapper-for-fstat-for-when-you-just-want-the-s.patch application/octet-stream 1.6 KB
0002-Use-pg_pread-and-pg_pwrite-in-slru.c.patch application/octet-stream 3.6 KB
0003-Use-pg_fstat_size-in-walsender.c.patch application/octet-stream 1.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Arseny Sher 2020-02-11 05:06:59 Re: ERROR: subtransaction logged without previous top-level txn record
Previous Message Arseny Sher 2020-02-11 04:32:22 Re: ERROR: subtransaction logged without previous top-level txn record