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 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. -- Michael
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e88d545d65..bbd6311b35 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -695,11 +695,12 @@ pgarch_archiveXlog(char *xlog)
* 2) because the oldest ones will sooner become candidates for
* recycling at time of checkpoint
*
- * NOTE: the "oldest" comparison will presently consider all segments of
- * a timeline with a smaller ID to be older than all segments of a timeline
- * with a larger ID; the net result being that past timelines are given
- * higher priority for archiving. This seems okay, or at least not
- * obviously worth changing.
+ * NOTE: the "oldest" comparison will consider any .history file to be older
+ * than any other file except another .history file. Segments on a timeline
+ * with a smaller ID will be older than all segments on a timeline with a
+ * larger ID; the net result being that past timelines are given higher
+ * priority for archiving. This seems okay, or at least not obviously worth
+ * changing.
*/
static bool
pgarch_readyXlog(char *xlog)
@@ -715,6 +716,7 @@ pgarch_readyXlog(char *xlog)
DIR *rldir;
struct dirent *rlde;
bool found = false;
+ bool historyFound = false;
snprintf(XLogArchiveStatusDir, MAXPGPATH, XLOGDIR "/archive_status");
rldir = AllocateDir(XLogArchiveStatusDir);
@@ -722,32 +724,54 @@ pgarch_readyXlog(char *xlog)
while ((rlde = ReadDir(rldir, XLogArchiveStatusDir)) != NULL)
{
int basenamelen = (int) strlen(rlde->d_name) - 6;
+ char basename[MAX_XFN_CHARS + 1];
+ bool ishistory;
- if (basenamelen >= MIN_XFN_CHARS &&
- basenamelen <= MAX_XFN_CHARS &&
- strspn(rlde->d_name, VALID_XFN_CHARS) >= basenamelen &&
- strcmp(rlde->d_name + basenamelen, ".ready") == 0)
+ /* Ignore entries with unexpected number of characters */
+ if (basenamelen < MIN_XFN_CHARS ||
+ basenamelen > MAX_XFN_CHARS)
+ continue;
+
+ /* Ignore entries with unexpected characters */
+ if (strspn(rlde->d_name, VALID_XFN_CHARS) < basenamelen)
+ continue;
+
+ /* Ignore anything not suffixed with .ready */
+ if (strcmp(rlde->d_name + basenamelen, ".ready") != 0)
+ continue;
+
+ /* Truncate off the .ready */
+ memcpy(basename, rlde->d_name, basenamelen);
+ basename[basenamelen] = '\0';
+
+ /* Is this a history file? */
+ ishistory = IsTLHistoryFileName(basename);
+
+ /*
+ * Consume the file to archive. History files have the highest
+ * priority. If this is the first file or the first history file
+ * ever, copy it. In the presence of a history file already chosen as
+ * target, ignore all other files except history files which have been
+ * generated for an older timeline than what is already chosen as
+ * target to archive.
+ */
+ if (!found || (ishistory && !historyFound))
{
- if (!found)
- {
- strcpy(newxlog, rlde->d_name);
- found = true;
- }
- else
- {
- if (strcmp(rlde->d_name, newxlog) < 0)
- strcpy(newxlog, rlde->d_name);
- }
+ strcpy(newxlog, basename);
+ found = true;
+ historyFound = ishistory;
+ }
+ else if (ishistory || (!ishistory && !historyFound))
+ {
+ if (strcmp(basename, newxlog) < 0)
+ strcpy(newxlog, basename);
}
}
FreeDir(rldir);
if (found)
- {
- /* truncate off the .ready */
- newxlog[strlen(newxlog) - 6] = '\0';
strcpy(xlog, newxlog);
- }
+
return found;
}
signature.asc
Description: PGP signature
