From: | David Steele <david(at)pgmasters(dot)net> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Timeline ID in backup_label file |
Date: | 2017-11-16 00:20:38 |
Message-ID: | 2437aebc-410d-1ad8-1729-b4c62e72978e@pgmasters.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/15/17 6:01 PM, Michael Paquier wrote:
> On Wed, Nov 15, 2017 at 11:16 PM, David Steele <david(at)pgmasters(dot)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?
Yes, those names are 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.
For this patch at least, I think we should do #1. Getting rid of the
order dependency is attractive but there may be other programs that are
depending on the order. I know you are not proposing to change the
order now, but it *could* be changed with #2.
Also, I think DEBUG1 would be a more appropriate log level if any
logging is done.
Regards,
--
-David
david(at)pgmasters(dot)net
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2017-11-16 00:32:40 | Re: [HACKERS] Partition-wise aggregation/grouping |
Previous Message | Jason Petersen | 2017-11-16 00:12:20 | Re: pspg - psql pager |