From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | michael(at)paquier(dot)xyz |
Cc: | rjuju123(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: standby recovery fails (tablespace related) (tentative patch and discussion) |
Date: | 2022-03-04 06:30:57 |
Message-ID: | 20220304.153057.358077020395354462.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks to look this!
At Fri, 4 Mar 2022 13:51:12 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote i
n
> On Fri, Mar 04, 2022 at 09:10:48AM +0900, Kyotaro Horiguchi wrote:
> > And same function contained a maybe-should-have-been-removed line
> > which makes Windows build unhappy.
> >
> > This should make all platforms in the CI happy.
>
> d6d317d as solved the issue of tablespace paths across multiple nodes
> with the new GUC called allow_in_place_tablespaces, and is getting
> successfully used in the recovery tests as of 027_stream_regress.pl.
The feature allows only one tablespace directory. but that uses (I'm
not sure it needs, though) multiple tablespace directories so I think
the feature doesn't work for the test.
Maybe I'm missing something, but it doesn't use tablespaces. I see
that in 002_tablespace.pl but but the test uses only one tablespace
location.
> Shouldn't we rely on that rather than extending more our test perl
> modules? One tricky part is the emulation of readlink for junction
> points on Windows (dir_readlink in your patch), and the root of the
Yeah, I don't like that as I said before...
> problem is that 0003 cares about the path structure of the
> tablespaces so we have no need, as far as I can see, for any
> dependency with link follow-up in the scope of this patch.
I'm not sure how this related to 0001 but maybe I don't follow this.
> This means that you should be able to simplify the patch set, as we
> could entirely drop 0001 in favor of enforcing the new dev GUC in the
> nodes created in the TAP test of 0002.
Maybe it's possible by breaking the test into ones that need only one
tablespace. I'll give it a try.
> Speaking of 0002, perhaps this had better be in its own file rather
> than extending more 011_crash_recovery.pl. 0003 looks like a good
Ok, no problem.
> idea to check after the consistency of the path structures created
> during replay, and it touches paths I'd expect it to touch, as of
> database and tbspace redos.
>
> + if (!reachedConsistency)
> + XLogForgetMissingDir(xlrec->ts_id, InvalidOid);
> +
> + XLogFlush(record->EndRecPtr);
> Not sure to understand why this is required. A comment may be in
> order to explain the hows and the whys.
Is it about XLogFlush? As my understanding it is to update
minRecoveryPoint to that LSN. I'll add a comment like that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2022-03-04 06:44:22 | pg_tablespace_location() failure with allow_in_place_tablespaces |
Previous Message | Amit Kapila | 2022-03-04 06:21:41 | Re: Add the replication origin name and commit-LSN to logical replication worker errcontext |