On Wed, Nov 15, 2017 at 11:16 PM, David Steele <david@pgmasters.net> wrote:
> Find my review below.
>
> On 10/26/17 2:03 PM, Michael Paquier wrote:
>>
>> Thanks for the feedback. Attached is a patch to achieve so, I have
>> added as well a STOP TIMELINE field in the backup history file. Note
>> that START TIMELINE gets automatically into the backup history file.
>> Added a CF entry as well.
> + TimeLineID tli1, tli2;
>
> I'm not that excited about these names but don't have any better ideas.
One is tli_from_segname and the second is tli_from_file. Would
something like that sound better?
> + if (fscanf(lfp, "START TIMELINE: %u\n", &tli2) == 1)
>
> This didn't work when I tested it (I had intentionally munged the "START
> TIMELINE" to produce an error). The problem is the "START TIME" and
> "LABEL" lines which are not being read. I added a few fgets() calls and
> it worked.
Currently backup label files are generated as such:
START WAL LOCATION: 0/2000028 (file 000000010000000000000002)
CHECKPOINT LOCATION: 0/2000060
BACKUP METHOD: streamed
BACKUP FROM: master
START TIME: 2017-11-16 07:52:50 JST
LABEL: popo
START TIMELINE: 1
There could be two things that could be done here:
1) Keep read_backup_label order-dependent and look at the START TIME
and LABEL fields, then kick in ereport(LOG) with the information. This
makes the fields in the backup_label still order-dependent.
2) Make read_backup_label smarter by parsing it line-by-line and fill
in the data wanted. This way the parsing logic and sanity checks are
split into two, and Postgres is smarter at looking at backup_label
files generated by any backup tool.
Which one do you prefer? Getting input from backup tool maintainers is
important here. 2) is more extensible if more fields are added to the
backup_label for a reason or another in the future.
--
Michael
