Date: Tue, 29 Mar 2022 13:55:35 +0900 (JST)
Message-Id: <20220329.135535.310000883119475499.horikyota.ntt@gmail.com>
To: robertmhaas@gmail.com
Cc: alvherre@alvh.no-ip.org, michael@paquier.xyz, rjuju123@gmail.com,
 pgsql-hackers@postgresql.org
Subject: Re: standby recovery fails (tablespace related) (tentative patch
 and discussion)
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: 
 <CA+TgmoYcUPL+WOJL2ZzhH=zmrhj0iOQ=iCFM0SuYqBbqZEamEg@mail.gmail.com>
References: 
 <CA+TgmoZ2B=s+9UKmxeNCEkV0Yu7X35NbeAc1TaCKvXRMDe=CbA@mail.gmail.com>
	<202203211903.ni5vfpmyelk7@alvherre.pgsql>
	<CA+TgmoYcUPL+WOJL2ZzhH=zmrhj0iOQ=iCFM0SuYqBbqZEamEg@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220329.135535.310000883119475499.horikyota.ntt%40gmail.com>
Precedence: bulk

At Mon, 28 Mar 2022 12:17:50 -0400, Robert Haas <robertmhaas@gmail.com> wrote in 
> On Mon, Mar 21, 2022 at 3:02 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > > 2. Why not instead change the code so that the operation can succeed,
> > > by creating the prerequisite parent directories? Do we not have enough
> > > information for that? I'm not saying that we definitely should do it
> > > that way rather than this way, but I think we do take that approach in
> > > some cases.
> >
> > It seems we can choose freely between these two implementations -- I
> > mean I don't see any upsides or downsides to either one.
> 
> What got committed here feels inconsistent to me. Suppose we have a
> checkpoint, and then a series of operations that touch a tablespace,
> and then a drop database and drop tablespace. If the first operation
> happens to be CREATE DATABASE, then this patch is going to fix it by
> skipping the operation. However, if the first operation happens to be
> almost anything else, the way it's going to reference the dropped
> tablespace is via a block reference in a WAL record of a wide variety
> of types. That's going to result in a call to
> XLogReadBufferForRedoExtended() which will call
> XLogReadBufferExtended() which will do smgrcreate(smgr, forknum, true)
> which will in turn call TablespaceCreateDbspace() to fill in all the
> missing directories.

Right. I thought that recovery avoids that but that's wrong.  This
behavior creates a bare (non-linked) directly within pg_tblspc.  The
directory would dissapear soon if recovery proceeds to the consistency
point, though.

> I don't think that's very good. It would be reasonable to decide that
> we're never going to create the missing directories and instead just
> remember that they were not found so we can do a cross check. It's
> also reasonable to just create the directories on the fly. But doing a
> mix of those systems doesn't really seem like the right idea -
> particularly because it means that the cross-check system is probably
> not very effective at finding actual problems in the code.
> 
> Am I missing something here?

No. I agree that mixing them is not good.  On the other hand we
already doing that by heapam.  AFAICS sometimes it avoid creating a
new page but sometimes creates it.  But I don't mean to use the fact
for justifying this patch to do that, or denying to do that.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center