agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: Attempt to consolidate reading of XLOG page
Date: Fri, 22 Nov 2019 16:55:20 +0100
Message-ID: <10180.1574438120@antos> (raw)
In-Reply-To: <20191122142528.GA6465@alvherre.pgsql>
References: <20191122142528.GA6465@alvherre.pgsql>

Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

> On 2019-Nov-22, Michael Paquier wrote:
> 
> > On Fri, Nov 22, 2019 at 10:35:51AM -0300, Alvaro Herrera wrote:
> > > FWIW I think the new code is buggy because it doesn't seem to be setting
> > > ws_off, so I suppose the optimization in ReadPageInternal to skip
> > > reading the page when it's already the page we have is not hit, except
> > > for the first page in the segment.  I didn't verify this, just my
> > > impression while reading the code.
> > 
> > FWIW, this matches with my impression here, third paragraph:
> > https://www.postgresql.org/message-id/20191120083802.GB47145@paquier.xyz
> 
> Ah, right.

As I pointed out in

https://www.postgresql.org/message-id/88183.1574261429%40antos

seg.ws_off only replaced readOff in XLogReaderState. So we should only update
ws_off where readOff was updated before commit 709d003. This does happen in
ReadPageInternal (see HEAD) and I see no reason for the final patch to update
ws_off anywhere else.

> I was wondering if we shouldn't do away with the concept of "offset" as
> such, since the offset there is always forcibly set to the start of a
> page.  Why don't we count page numbers instead?  It seems like the
> interface is confusingly generic (measure in bytes) yet not offer any
> extra functionality that could not be obtained with a simpler struct
> repr (measure in pages).

Yes, I agree that page numbers would be sufficient.

> But then that's not something that we need to change in this patch.

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





view thread (62+ messages)  latest in thread

Message-ID: <10180.1574438120@antos>
Permalink:  ../10180.1574438120@antos/
Also on:    postgresql.org/message-id/10180.1574438120@antos

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: ah@cybertec.at, alvherre@2ndquadrant.com, michael@paquier.xyz, thomas.munro@gmail.com, robertmhaas@gmail.com
  Subject: Re: Attempt to consolidate reading of XLOG page
  In-Reply-To: <10180.1574438120@antos>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox