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

Hello.

FWIW it seems to me a bug that making an inconsistent set of
files in archive directory.

At Fri, 21 Dec 2018 12:19:21 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20181221031921.GE1886@paquier.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.

> also did not like the high-jacking of rlde->d_name so instead let's use
> an intermediate variable to store the basename of a scanned entry.  The
> format of the if/elif with comments in-between was not really consistent
> with the common practice as well.  pg_indent has also been applied.
> 
> > Ah, I see.  Yes, that's exactly how I tested it, in addition to doing real
> > promotions.
> 
> OK, so am I doing.
> 
> Attached is an updated patch.  Does that look fine to you?  The base
> logic is unchanged, and after a promotion history files get archived
> before the last partial segment.

Renaming history to ishistory looks good.

if (!found || (ishistory && !historyFound))
{
  /* init */
  found = true;
  historyFound = ishistory;
}
else if (ishistory || (!ishstory && !historyFound))
 /* compare/replace */

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)

"else if (!historyFound || ishistory)"



> 		strcpy(xlog, newxlog);

The caller prepares sufficient memory for basename, and we no
longer copy ".ready" into newxlog. Douldn't we work directly on
xlog instead of allocating newxlog?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center