Re: Change pgarch_readyXlog() to return .history files first

At Fri, 21 Dec 2018 14:17:25 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20181221051724(dot)GG1886(at)paquier(dot)xyz>
> On Fri, Dec 21, 2018 at 01:49:18PM +0900, Kyotaro HORIGUCHI wrote:
> > FWIW it seems to me a bug that making an inconsistent set of
> > files in archive directory.
>
> Okay, point taken! FWIW, I have no actual objections in not
> back-patching that.

I maybe(?) know.

> > At Fri, 21 Dec 2018 12:19:21 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20181221031921(dot)GE1886(at)paquier(dot)xyz>
> >> On Thu, Dec 20, 2018 at 01:57:30PM +0200, David Steele wrote:
> >> > Good point. The new patch uses IsTLHistoryFileName().
> >>
> >> OK, I have been reviewing the patch and the logic is correct, still I
> >> could not resist reducing the number of inner if's for readability. I
> >
> > +1 basically. But I think that tail(name, 6) != ".ready" can
> > happen with a certain frequency but strspn(name) < basenamelen
> > rarely in the normal case.
>
> So that +0.5 if "basically" means a partial agreement? :p

Mmm. No, +0.9.

> > In the else if condition, ishisotry must be false in the right
> > hand of ||. What we do here is ignoring non-history files once
> > history file found. (Just a logic condensing and it would be done
> > by compiler, though)
>
> Yes, this can be simplified. So let's do so.
>
> > The caller prepares sufficient memory for basename, and we no
> > longer copy ".ready" into newxlog. Wouldn't we work directly on

Sorry for silly typo, but the 'W' was 'C', I meant:p

> > xlog instead of allocating newxlog?
>
> Okay, let's simplify that as you suggest.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center