Re: Attempt to consolidate reading of XLOG page

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: 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: Sat, 28 Sep 2019 15:00:35 +0200
Message-ID: <9236.1569675635@antos> (raw)
In-Reply-To: <7753.1569658465@antos>
References: <20190927191736.GA13447@alvherre.pgsql>
	<7753.1569658465@antos>

Antonin Houska <ah@cybertec.at> wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> 
> > BTW that tli_p business to the openSegment callback is horribly
> > inconsistent.  Some callers accept a NULL tli_p, others will outright
> > crash, even though the API docs say that the callback must determine the
> > timeline.  This is made more complicated by us having the TLI in "seg"
> > also.  Unless I misread, the problem is again that the walsender code is
> > doing nasty stuff with globals (endSegNo).  As a very minor stylistic
> > point, we prefer to have out params at the end of the signature.
> 
> XLogRead() tests for NULL so it should not crash but I don't insist on doing
> it this way. XLogRead() actually does not have to care whether the "open
> segment callback" determines the TLI or not, so it (XLogRead) can always
> receive a valid pointer to seg.ws_tli.

This is actually wrong - seg.ws_tli is not always the correct value to
pass. If seg.ws_tli refers to the segment from which data was read last time,
then XLogRead() still needs a separate argument to specify from which TLI the
current call should read. If these two differ, new file needs to be opened.

The problem of walsender.c is that its implementation of XLogRead() does not
care about the TLI of the previous read. If the behavior of the new, generic
implementation should be exactly the same, we need to tell XLogRead() that in
some cases it also should not compare the current TLI to the previous
one. That's why I tried to use the NULL pointer, or the InvalidTimeLineID
earlier.

Another approach is to add a boolean argument "check_tli", but that still
forces caller to pass some (random) value of the tli. The concept of
InvalidTimeLineID seems to me less disturbing than this.

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

view thread (62+ messages)  latest in thread

Message-ID: <9236.1569675635@antos>
Permalink:  ../9236.1569675635@antos/
Also on:    postgresql.org/message-id/9236.1569675635@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, thomas.munro@gmail.com, robertmhaas@gmail.com
  Subject: Re: Attempt to consolidate reading of XLOG page
  In-Reply-To: <9236.1569675635@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