From: Antonin Houska <ah@cybertec.at>
To: Michael Paquier <michael@paquier.xyz>
cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
    Thomas Munro <thomas.munro@gmail.com>,
    Robert Haas <robertmhaas@gmail.com>,
    "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Attempt to consolidate reading of XLOG page
In-reply-to: <20191122004903.GB42684@paquier.xyz>
References: <20191115214102.GA15616@alvherre.pgsql> <88183.1574261429@antos>
 <20191121080550.GG153437@paquier.xyz> <20191122004903.GB42684@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
   message dated "Fri, 22 Nov 2019 09:49:03 +0900."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <453.1574407712.1@antos>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 22 Nov 2019 08:28:32 +0100
Message-ID: <454.1574407712@antos>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> wrote:

> On Thu, Nov 21, 2019 at 05:05:50PM +0900, Michael Paquier wrote:
> > And with WAL segments at 1MB, I was seeing quite a slowdown with the
> > patch...  Then I have done an extra test with pg_waldump with the
> > segments generated previously with the output redirected to /dev/null.
> > Going through 512 segments takes 15.730s with HEAD (average of 3 runs)
> > and 15.851s with the patch.
> =

> Here are more tests with pg_waldump and 1MB/1GB segment sizes with
> records generated from pgbench, (7 runs, eliminated the two highest
> and two lowest, these are the remaining 3 runs as real time):
> 1) 1MB segment size, 512 segments:
> time pg_waldump 000000010000000100000C00 000000010000000100000F00 > /dev=
/null
> - HEAD: 0m4.512s, 0m4.446s, 0m4.501s
> - Patch + system's pg_read: 0m4.495s, 0m4.502s, 0m4.486s
> - Patch + fallback pg_read: 0m4.505s, 0m4.527s, 0m4.495s
> 2) 1GB segment size, 3 segments:
> time pg_waldump 000000010000000200000001 000000010000000200000003 > /dev=
/null
> - HEAD: 0m11.802s, 0m11.834s, 0m11.846s
> - Patch + system's pg_read: 0m11.939s, 0m11.991s, 0m11.966s
> - Patch + fallback pg_read: 0m12.054s, 0m12.066s, 0m12.159s
> So there is a tendency for a small slowdown here.  Still it is not
> that much, so I withdraw my concerns.

Thanks for the testing!

I thought that in [1] you try discourage me from using pg_pread(), but now=
 it
seems to be the opposite. Ideally I'd like to see no overhead added by my
patch at all, but the code simplicity should matter too.

As a clue, we can perhaps consider the fact that commit c24dcd0c removed
explicit lseek() also from XLogWrite(), but I'm not sure how much we can
compare XLOG writing and reading (I'd expect writing to be a bit less
sequential than reading because XLogWrite() may need to write the last pag=
e
more than once.)

Let's wait for Alvaro's judgement.

> Another thing:
> +void       WALReadRaiseError(WALReadError *errinfo);
> This is missing an "extern" declaration.

I'll fix this as well as the other problem reported in [1] as soon as I kn=
ow
whether pg_pread() should be used or not.

[1] https://www.postgresql.org/message-id/20191121080550.GG153437%40paquie=
r.xyz

-- =

Antonin Houska
Web: https://www.cybertec-postgresql.com