> I have a few suggestions on the patch
> 1.
> +
> + /*
> + * Found the oldest WAL, reset timeline ID and log segment number to
> generate
> + * the next WAL file in the sequence.
> + */
> + if (found && !historyFound)
> + {
> + XLogFromFileName(xlog, &curFileTLI, &nextLogSegNo, wal_segment_size);
> + ereport(LOG,
> + (errmsg("directory scan to archive write-ahead log file \"%s\"",
> + xlog)));
> + }
>
> If a history file is found we are not updating curFileTLI and
> nextLogSegNo, so it will attempt the previously found segment.  This
> is fine because it will not find that segment and it will rescan the
> directory.  But I think we can do better, instead of searching the
> same old segment in the previous timeline we can search that old
> segment in the new TL so that if the TL switch happened within the
> segment then we will find the segment and we will avoid the directory
> search.
>
>
>  /*
> + * Log segment number and timeline ID to get next WAL file in a sequence.
> + */
> +static XLogSegNo nextLogSegNo = 0;
> +static TimeLineID curFileTLI = 0;
> +
>
> So everytime archiver will start with searching segno=0 in timeline=0.
> Instead of doing this can't we first scan the directory and once we
> get the first segment to archive then only we can start predicting the
> next wal segment?  I think there is nothing wrong even if we try to
> look for seg 0 in timeline 0, everytime we start the archivar but that
> will be true only once in the history of the cluster so why not skip
> this until we scan the directory once?
>

+1, I like Dilip's ideas here to optimize further.

Also, one minor comment:

+   /*
+    * Log segment number already points to the next file in the sequence

+    * (as part of successful archival of the previous file). Generate the
path
+    * for status file.

+    */

This comment is a bit confusing with the name of the variable nextLogSegNo.
I think the name of the variable is appropriate here, but maybe we can
reword
the comment something like:

+       /*
+        * We already have the next anticipated log segment number and the
+        * timeline, check if this WAL file is ready to be archived. If
yes, skip
+        * the directory scan.
+        */

Regards,
Jeevan Ladhe

Reply via email to