From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: ah@cybertec.at
Cc: alvherre@2ndquadrant.com
Cc: thomas.munro@gmail.com
Cc: robertmhaas@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: Attempt to consolidate reading of XLOG page
Date: Tue, 01 Oct 2019 12:22:27 +0900 (Tokyo Standard Time)
Message-ID: <20191001.122227.209046280.horikyota.ntt@gmail.com> (raw)
In-Reply-To: <9236.1569675635@antos>
References: <20190927191736.GA13447@alvherre.pgsql>
<7753.1569658465@antos>
<9236.1569675635@antos>
Hello,
At Sat, 28 Sep 2019 15:00:35 +0200, Antonin Houska <ah@cybertec.at> wrote in <9236.1569675635@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.
openSegment represents the file *currently* opened. XLogRead
needs the TLI *to be* opened. If they are different, as far as
wal logical wal sender and pg_waldump is concerned, XLogRead
switches to the new TLI and the new TLI is set to
openSegment.ws_tli. So, it seems to me that the parameter
doesn't need to be inout? It is enough that it is an "in"
parameter.
> 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.
Physical wal sender doesn't switch TLI. So I don't think the
behavior doesn't harm (or doesn't fire). openSegment holds the
TLI set at the first call. (Even if future wal sender switches
TLI, the behavior should be needed.)
> 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.
So I think InvalidTimeLineID is not needed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
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: horikyota.ntt@gmail.com, 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: <20191001.122227.209046280.horikyota.ntt@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox