Re: [HACKERS] Timeline ID in backup_label file

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: David Steele <david(at)pgmasters(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [HACKERS] Timeline ID in backup_label file
Date: 2018-01-06 00:54:35
Message-ID: CAB7nPqRb1X820rfvs8uNK4TnpmyvdYaY-dZ40v21jpRN3=YmBQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Fri, Jan 5, 2018 at 11:13 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 27 November 2017 at 14:06, David Steele <david(at)pgmasters(dot)net> wrote:
>
>> I have tested and get an error as expected when I munge the backup_label
>> file:
>>
>> FATAL: invalid data in file "backup_label"
>> DETAIL: Timeline ID parsed is 2, but expected 1
>>
>> Everything else looks good so I will mark it ready for committer.
>
> Sounds good.

Thanks for the feedback.

> No tests?

Do you think that's worth the cycles as a TAP test? This can be done
by generating a dummy backup_label file and then start a standby to
see the failure, but this looks quite costly for minimum gain. This
code path is also taken tested in
src/test/recovery/t/001_stream_rep.pl for example, though that's not
the failure path. So if we add a DEBUG1 line after fetching correctly
the timeline ID this could help by looking at
tmp_check/log/001_stream_rep_standby_1.log, but this needs to set
"log_min_messages = debug1" PostgresNode.pm for example so as this
shows up in the logs (this could be useful if done by default
actually, DEBUG1 is not too talkative). Attached is an example of
patch doing so. See for the addition in PostgresNode.pm and this new
bit:
+ ereport(DEBUG1,
+ (errmsg("backup timeline %u in file \"%s\"",
+ tli_from_file, BACKUP_LABEL_FILE)));

> No docs/extended explanatory comments?

There is no existing documentation about the format of the backup_file
in doc/. So it seems to me that it is not the problem of this patch to
fill in the hole. Regarding the comments, perhaps something better
could be done, but I am not quite sure what. We don't much explain
what the current fields mean, except that one can guess what they mean
from their names, which is the intention behind the code.
--
Michael

Attachment Content-Type Size
backup_label_tli_v2.patch application/octet-stream 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-01-06 00:56:03 Re: [HACKERS] Timeline ID in backup_label file
Previous Message Michael Paquier 2018-01-06 00:10:51 Re: pgsql: Implement channel binding tls-server-end-point for SCRAM