Re: pread() and pwrite()

On Thu, Nov 8, 2018 at 4:27 AM Andrew Dunstan wrote: > On 11/7/18 10:05 AM, Jesper Pedersen wrote: > > On 11/7/18 9:30 AM, Tom Lane wrote: > >> I'm confused by this. Surely the pwrite-based code is writing > >> exactly the > >> same data as before. Do we have to conclude that valgrind is > >> co

Re: pread() and pwrite()

On 11/7/18 10:05 AM, Jesper Pedersen wrote: Hi Tom, On 11/7/18 9:30 AM, Tom Lane wrote: I'm confused by this.  Surely the pwrite-based code is writing exactly the same data as before.  Do we have to conclude that valgrind is complaining about passing uninitialized data to pwrite() when it d

Re: pread() and pwrite()

Hi Tom, On 11/7/18 9:30 AM, Tom Lane wrote: I'm confused by this. Surely the pwrite-based code is writing exactly the same data as before. Do we have to conclude that valgrind is complaining about passing uninitialized data to pwrite() when it did not complain about exactly the same thing for

Re: pread() and pwrite()

On 11/7/18 9:30 AM, Tom Lane wrote: Andrew Dunstan writes: On 11/7/18 7:26 AM, Jesper Pedersen wrote: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-11-07%2001%3A01%3A01 And lousyjack, which uses a slightly different way of calling valgrind, and thus got past initdb,

Re: pread() and pwrite()

Andrew Dunstan writes: > On 11/7/18 7:26 AM, Jesper Pedersen wrote: >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-11-07%2001%3A01%3A01 > And lousyjack, which uses a slightly different way of calling valgrind, > and thus got past initdb, found a bunch more: >

Re: pread() and pwrite()

Hi, On 11/7/18 7:26 AM, Jesper Pedersen wrote: On 11/6/18 4:04 PM, Thomas Munro wrote: On Wed, Nov 7, 2018 at 4:42 AM Jesper Pedersen Thanks!  Pushed.  I'll keep an eye on the build farm to see if anything breaks on Cygwin or some other frankenOS. There is [1] on Andres' skink setup. Looking

Re: pread() and pwrite()

On 11/7/18 7:26 AM, Jesper Pedersen wrote: Hi Thomas, On 11/6/18 4:04 PM, Thomas Munro wrote: On Wed, Nov 7, 2018 at 4:42 AM Jesper Pedersen Thanks!  Pushed.  I'll keep an eye on the build farm to see if anything breaks on Cygwin or some other frankenOS. There is [1] on Andres' skink setup

Re: pread() and pwrite()

Hi Thomas, On 11/6/18 4:04 PM, Thomas Munro wrote: On Wed, Nov 7, 2018 at 4:42 AM Jesper Pedersen Thanks! Pushed. I'll keep an eye on the build farm to see if anything breaks on Cygwin or some other frankenOS. There is [1] on Andres' skink setup. Looking. [1] https://buildfarm.postgresql.

Re: pread() and pwrite()

On Wed, Nov 7, 2018 at 4:42 AM Jesper Pedersen wrote: > Passes check-world, and includes the feedback on this thread. > > New status: Ready for Committer Thanks! Pushed. I'll keep an eye on the build farm to see if anything breaks on Cygwin or some other frankenOS. -- Thomas Munro http://www.

Re: pread() and pwrite()

Hi, On 11/5/18 9:10 PM, Thomas Munro wrote: On Tue, Nov 6, 2018 at 5:07 AM Alvaro Herrera wrote: Please remove Tell from line 18 in fd.h. To Küssnacht with him! Thanks, done. But what is this arrow sticking through my Mac laptop's screen...? On Tue, Nov 6, 2018 at 6:23 AM Tom Lane wrote:

Re: pread() and pwrite()

Thomas Munro writes: > On Tue, Nov 6, 2018 at 6:23 AM Tom Lane wrote: >> What I suggest is that we *not* try to make this a completely transparent >> substitute. Instead, make the functions exported by src/port/ be >> "pg_pread" and "pg_pwrite", ... > OK. But since we're using this from both f

Re: pread() and pwrite()

On Tue, Nov 6, 2018 at 5:07 AM Alvaro Herrera wrote: > Please remove Tell from line 18 in fd.h. To Küssnacht with him! Thanks, done. But what is this arrow sticking through my Mac laptop's screen...? On Tue, Nov 6, 2018 at 6:23 AM Tom Lane wrote: > Alvaro Herrera writes: > > On 2018-Nov-04,

Re: pread() and pwrite()

Alvaro Herrera writes: > On 2018-Nov-04, Thomas Munro wrote: >> Here's a patch to add Windows support by supplying >> src/backend/port/win32/pread.c. Thoughts? > Hmm, so how easy is to detect that somebody runs read/write on fds where > pread/pwrite have occurred? I guess for data files it's ea

Re: pread() and pwrite()

Please remove Tell from line 18 in fd.h. To Küssnacht with him! -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pread() and pwrite()

On 2018-Nov-04, Thomas Munro wrote: > Here's a patch to add Windows support by supplying > src/backend/port/win32/pread.c. Thoughts? Hmm, so how easy is to detect that somebody runs read/write on fds where pread/pwrite have occurred? I guess for data files it's easy to detect since you'd quickl

Re: pread() and pwrite()

ll, and then we can get rid of the conditional macro stuff at various call sites and use pread() and pwrite() freely. Here's a version that does it that way. One question is whether the caveat mentioned in patch 0001 is acceptable. Passed check-world, but I can't verify the 0001 patch

Re: pread() and pwrite()

UX 10.20 as well, and then we can get rid of the conditional macro stuff at various call sites and use pread() and pwrite() freely. Here's a version that does it that way. One question is whether the caveat mentioned in patch 0001 is acceptable. -- Thomas Munro http://www.enterprisedb.

Re: pread() and pwrite()

On Sat, Nov 3, 2018 at 2:07 AM Jesper Pedersen wrote: > This still applies, and passes make check-world. > > I wonder what the commit policy is on this, if the Windows part isn't > included. I read Heikki's comment [1] as it would be ok to commit > benefiting all platforms that has pread/pwrite.

Re: pread() and pwrite()

Hi Thomas, On 10/9/18 4:56 PM, Thomas Munro wrote: Thanks, much nicer. Rebased. This still applies, and passes make check-world. I wonder what the commit policy is on this, if the Windows part isn't included. I read Heikki's comment [1] as it would be ok to commit benefiting all platforms

Re: pread() and pwrite()

On Tue, Oct 9, 2018 at 5:08 PM Tom Lane wrote: > I wrote: > > Thomas Munro writes: > >> On Tue, Oct 9, 2018 at 2:55 PM Tom Lane wrote: > >>> Yeah, I've been burnt by that too recently. It occurs to me we could make > >>> that at least a little less painful if we formatted the macro with one > >

Re: pread() and pwrite()

On 10/09/2018 02:37 PM, Andres Freund wrote: On 2018-10-09 14:32:29 -0400, Andrew Dunstan wrote: On 10/08/2018 09:55 PM, Tom Lane wrote: Thomas Munro writes: Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! Yeah, I've been burnt by that too recently. It occurs to me

Re: pread() and pwrite()

On 2018-10-09 14:32:29 -0400, Andrew Dunstan wrote: > > > On 10/08/2018 09:55 PM, Tom Lane wrote: > > Thomas Munro writes: > > > Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! > > Yeah, I've been burnt by that too recently. It occurs to me we could make > > that at least a

Re: pread() and pwrite()

On 10/08/2018 09:55 PM, Tom Lane wrote: Thomas Munro writes: Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! Yeah, I've been burnt by that too recently. It occurs to me we could make that at least a little less painful if we formatted the macro with one line per functi

Re: pread() and pwrite()

I wrote: > Thomas Munro writes: >> On Tue, Oct 9, 2018 at 2:55 PM Tom Lane wrote: >>> Yeah, I've been burnt by that too recently. It occurs to me we could make >>> that at least a little less painful if we formatted the macro with one >>> line per function name: >> +1, was about to suggest the

Re: pread() and pwrite()

Thomas Munro writes: > On Tue, Oct 9, 2018 at 2:55 PM Tom Lane wrote: >> Yeah, I've been burnt by that too recently. It occurs to me we could make >> that at least a little less painful if we formatted the macro with one >> line per function name: >> >> AC_CHECK_FUNCS([ >> cbrt >> clock_gettime

Re: pread() and pwrite()

On Tue, Oct 9, 2018 at 2:55 PM Tom Lane wrote: > Thomas Munro writes: > > Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! > > Yeah, I've been burnt by that too recently. It occurs to me we could make > that at least a little less painful if we formatted the macro with one >

Re: pread() and pwrite()

Thomas Munro writes: > Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! Yeah, I've been burnt by that too recently. It occurs to me we could make that at least a little less painful if we formatted the macro with one line per function name: AC_CHECK_FUNCS([ cbrt

Re: pread() and pwrite()

On Fri, Sep 28, 2018 at 2:03 AM Jesper Pedersen wrote: > Thanks for v5 too. Rebased again. Patches that touch AC_CHECK_FUNCS are fun like that! -- Thomas Munro http://www.enterprisedb.com 0001-Use-pread-pwrite-instead-of-lseek-read-write-v6.patch Description: Binary data

Re: pread() and pwrite()

Hi Thomas, On 9/18/18 9:48 PM, Thomas Munro wrote: It certainly wouldn't hurt... but more pressing to get this committed would be Windows support IMHO. I think the thing to do is to open files with the FILE_FLAG_OVERLAPPED flag, and then use ReadFile() and WriteFile() with an LPOVERLAPPED struc

Re: pread() and pwrite()

On Wed, Sep 19, 2018 at 1:48 PM Thomas Munro wrote: > On Fri, Sep 7, 2018 at 2:17 AM Jesper Pedersen > wrote: > > > This needs a rebase again. And again, due to the conflict with ppoll in AC_CHECK_FUNCS. -- Thomas Munro http://www.enterprisedb.com 0001-Use-pread-pwrite-instead-of-lseek-read-

Re: pread() and pwrite()

On Fri, Sep 7, 2018 at 2:17 AM Jesper Pedersen wrote: > > This needs a rebase again. Done. > Would it be of benefit to update these call sites > > * slru.c >- SlruPhysicalReadPage >- SlruPhysicalWritePage > * xlogutils.c >- XLogRead > * pg_receivewal.c >- FindStreamingStart > * r

Re: pread() and pwrite()

Hi, On 09/05/2018 02:42 PM, Jesper Pedersen wrote: On 07/26/2018 10:04 PM, Thomas Munro wrote: Done.  Rebased. This needs a rebase again. Would it be of benefit to update these call sites * slru.c - SlruPhysicalReadPage - SlruPhysicalWritePage * xlogutils.c - XLogRead * pg_receivew

Re: pread() and pwrite()

Hi, On 07/26/2018 10:04 PM, Thomas Munro wrote: Done. Rebased. This needs a rebase again. Once resolved the patch passes make check-world, and a strace analysis shows the associated read()/write() have been turned into pread64()/pwrite64(). All lseek()'s are SEEK_END's. Best regards, J

Re: pread() and pwrite()

On Sat, Jul 21, 2018 at 3:34 AM Tom Lane wrote: > Heikki Linnakangas writes: > > No objections, if you want to make the effort. But IMHO the lseek+read > > fallback is good enough on Windows. Unless you were thinking that we > > could then remove the !HAVE_PREAD fallback altogether. Are there any

Re: pread() and pwrite()

On Fri, Jul 20, 2018 at 8:14 PM, Oskari Saarenmaa wrote: > On Thu, Jul 12, 2018 at 01:55:31PM +1200, Thomas Munro wrote: >> [...] I'm not even sure I'd bother adding "At" to the >> function names. If there are any extensions that want the old >> interface they will fail to compile either way. N

Re: pread() and pwrite()

> On 20 Jul 2018, at 17:34, Tom Lane wrote: > > Heikki Linnakangas writes: >> No objections, if you want to make the effort. But IMHO the lseek+read >> fallback is good enough on Windows. Unless you were thinking that we >> could then remove the !HAVE_PREAD fallback altogether. Are there any

Re: pread() and pwrite()

Heikki Linnakangas writes: > No objections, if you want to make the effort. But IMHO the lseek+read > fallback is good enough on Windows. Unless you were thinking that we > could then remove the !HAVE_PREAD fallback altogether. Are there any > other platforms out there that don't have pread/pwr

Re: pread() and pwrite()

On Thu, Jul 12, 2018 at 01:55:31PM +1200, Thomas Munro wrote: > A couple of years ago, Oskari Saarenmaa proposed a patch[1] to adopt > $SUBJECT. Last year, Tobias Oberstein argued again that we should do > that[2]. At the end of that thread there was a +1 from multiple > committers in support of

Re: pread() and pwrite()

On 20/07/18 01:50, Thomas Munro wrote: An idea for how to handle Windows, in a follow-up patch: add a file src/backend/port/win32/file.c that defines pgwin32_pread() and pgwin32_pwrite(), wrapping WriteFile()/ReadFile() and passing in an "OVERLAPPED" struct with the offset and sets errno on error

Re: pread() and pwrite()

On Thu, Jul 12, 2018 at 1:55 PM, Thomas Munro wrote: > I guess the only remaining reason to use FileSeek() is to get the file > size? So I wonder why SEEK_SET remains valid in the patch... if my > suspicion is correct that only SEEK_END still has a reason to exist, > perhaps we should just kill F

pread() and pwrite()

Hello hackers, A couple of years ago, Oskari Saarenmaa proposed a patch[1] to adopt $SUBJECT. Last year, Tobias Oberstein argued again that we should do that[2]. At the end of that thread there was a +1 from multiple committers in support of getting it done for PostgreSQL 12. Since Oskari hasn'