Alvaro Herrera <alvhe...@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.gb47...@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