At Wed, 29 Jan 2020 16:01:46 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in
> On 2020-01-28 06:01, Kyotaro Horiguchi wrote:
> > The code looks fine, renaming reachedStopPoint to
> > reachedRecoveryTarget looks very nice. Doc part looks fine, too.
> > PostgresNode.pm
> > +Is has_restoring is used, standby mode is used by default. To use
> > "Is has_restoring used,", or "If has_restoring is used," ?
>
> Committed with that fix.
Thanks.
> > The change seems aiming not to break compatibility with external test
> > scripts, but it looks quite confusing to me. The problem here is both
> > enable_streaming/restoring independently put trigger files, so don't
> > we separte placing of trigger files out of the functions?
>
> Yeah, this is all historically grown, but a major refactoring seems
> out of scope for this thread. It seems hard to come up with a more
> elegant way, since after all the underlying mechanisms are also all
> intertwined. Your patch adds even more code, so I'm not sure it's an
> improvement.
Yeah, as I wrote, I thogut that as too much, but I think at least POD
part for the internal-but-externally-used routines would be
needed. Don't we?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
