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 10:14:25 +0200
Message-ID: <7753.1569658465@antos> (raw)
In-Reply-To: <20190927191736.GA13447@alvherre.pgsql>
References: <20190927191736.GA13447@alvherre.pgsql>

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. However that in turn implies that
XLogRead() does not need the "tli" argument at all.

> > > Why do we leave this responsibility to ReadPageInternal?  Wouldn't it
> > > make more sense to leave XLogRead be always responsible for setting
> > > these correctly, and remove those lines from ReadPageInternal?
> > 
> > I think there's no rule that ReadPageInternal() must use XLogRead(). If we do
> > what you suggest, we need make this responsibility documented. I'll consider
> > that.

I think now we should not add any responsibility to XLogPageReadCB or its
subroutines because some extensions might already have their implementation of
XLogPageReadCB w/o XLogRead, and this change would break them.

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





view thread (62+ messages)  latest in thread

Message-ID: <7753.1569658465@antos>
Permalink:  ../7753.1569658465@antos/
Also on:    postgresql.org/message-id/7753.1569658465@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: <7753.1569658465@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