Re: [HACKERS] Creating backup history files for backups taken from standbys
On Tue, Sep 19, 2017 at 2:48 PM, Masahiko Sawada wrote: > On Tue, Sep 19, 2017 at 8:33 AM, David Steele wrote: >> On 9/18/17 7:26 PM, Michael Paquier wrote: >>> On Tue, Sep 19, 2017 at 8:14 AM, David Steele wrote: On 8/31/17 11:56 PM, Michael Paquier wrote: > Here is an updated patch with refreshed documentation, as a result of > 449338c which was discussed in thread > https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. > I am just outlining the fact that a backup history file gets created > on standbys and that it is archived. The patch looks good overall. It applies cleanly and passes all tests. One question. Do we want to create this file all the time (as written), or only when archive_mode = always? It appears that CleanupBackupHistory() will immediately remove the history file when archive_mode != always so it seems useless to write it. On the other hand the code is a bit simpler this way. Thoughts? >>> >>> With archive_mode = off, the bytes of the backup history file are >>> still written to disk, so my take on the matter is to keep the code >>> simple. >> >> I'm OK with that. >> >> I'll give Masahiko some time to respond before marking it RFC. >> > > Thanks, I'll review it and send a comment by this Wednesday. > I've reviewed and tested the latest patch but fond no problems, so I've marked this patch to Ready for Committer. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Tue, Sep 19, 2017 at 8:33 AM, David Steele wrote: > On 9/18/17 7:26 PM, Michael Paquier wrote: >> On Tue, Sep 19, 2017 at 8:14 AM, David Steele wrote: >>> On 8/31/17 11:56 PM, Michael Paquier wrote: Here is an updated patch with refreshed documentation, as a result of 449338c which was discussed in thread https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. I am just outlining the fact that a backup history file gets created on standbys and that it is archived. >>> >>> The patch looks good overall. It applies cleanly and passes all tests. >>> >>> One question. Do we want to create this file all the time (as written), >>> or only when archive_mode = always? >>> >>> It appears that CleanupBackupHistory() will immediately remove the >>> history file when archive_mode != always so it seems useless to write >>> it. On the other hand the code is a bit simpler this way. Thoughts? >> >> With archive_mode = off, the bytes of the backup history file are >> still written to disk, so my take on the matter is to keep the code >> simple. > > I'm OK with that. > > I'll give Masahiko some time to respond before marking it RFC. > Thanks, I'll review it and send a comment by this Wednesday. -- Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On 9/18/17 7:26 PM, Michael Paquier wrote: > On Tue, Sep 19, 2017 at 8:14 AM, David Steele wrote: >> On 8/31/17 11:56 PM, Michael Paquier wrote: >>> Here is an updated patch with refreshed documentation, as a result of >>> 449338c which was discussed in thread >>> https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. >>> I am just outlining the fact that a backup history file gets created >>> on standbys and that it is archived. >> >> The patch looks good overall. It applies cleanly and passes all tests. >> >> One question. Do we want to create this file all the time (as written), >> or only when archive_mode = always? >> >> It appears that CleanupBackupHistory() will immediately remove the >> history file when archive_mode != always so it seems useless to write >> it. On the other hand the code is a bit simpler this way. Thoughts? > > With archive_mode = off, the bytes of the backup history file are > still written to disk, so my take on the matter is to keep the code > simple. I'm OK with that. I'll give Masahiko some time to respond before marking it RFC. Regards, -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Tue, Sep 19, 2017 at 8:14 AM, David Steele wrote: > On 8/31/17 11:56 PM, Michael Paquier wrote: >> Here is an updated patch with refreshed documentation, as a result of >> 449338c which was discussed in thread >> https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. >> I am just outlining the fact that a backup history file gets created >> on standbys and that it is archived. > > The patch looks good overall. It applies cleanly and passes all tests. > > One question. Do we want to create this file all the time (as written), > or only when archive_mode = always? > > It appears that CleanupBackupHistory() will immediately remove the > history file when archive_mode != always so it seems useless to write > it. On the other hand the code is a bit simpler this way. Thoughts? With archive_mode = off, the bytes of the backup history file are still written to disk, so my take on the matter is to keep the code simple. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
Hi Michael, On 8/31/17 11:56 PM, Michael Paquier wrote: > Here is an updated patch with refreshed documentation, as a result of > 449338c which was discussed in thread > https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. > I am just outlining the fact that a backup history file gets created > on standbys and that it is archived. The patch looks good overall. It applies cleanly and passes all tests. One question. Do we want to create this file all the time (as written), or only when archive_mode = always? It appears that CleanupBackupHistory() will immediately remove the history file when archive_mode != always so it seems useless to write it. On the other hand the code is a bit simpler this way. Thoughts? Regards, -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Thu, Aug 10, 2017 at 4:55 PM, Masahiko Sawada wrote: > On Thu, Aug 10, 2017 at 4:50 PM, Michael Paquier > wrote: >> On Thu, Aug 10, 2017 at 9:45 AM, Masahiko Sawada >> wrote: >>> Thank you for the patch. Regarding to creating the backup history file >>> on stanbys, is there any difference from the patch posted on earlier >>> thread? >> >> That's a rebased version on top of what has been applied, except that >> I have fixed an incorrect comment and shuffled the code building the >> WAL segment name for the stop position. > > Understood, I'll review this patch. Here is an updated patch with refreshed documentation, as a result of 449338c which was discussed in thread https://www.postgresql.org/message-id/d4d951b9-89c0-6bc1-b6ff-d0b2dd5a8...@pgmasters.net. I am just outlining the fact that a backup history file gets created on standbys and that it is archived. -- Michael standby-backup-history-v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Thu, Aug 10, 2017 at 4:50 PM, Michael Paquier wrote: > On Thu, Aug 10, 2017 at 9:45 AM, Masahiko Sawada > wrote: >> Thank you for the patch. Regarding to creating the backup history file >> on stanbys, is there any difference from the patch posted on earlier >> thread? > > That's a rebased version on top of what has been applied, except that > I have fixed an incorrect comment and shuffled the code building the > WAL segment name for the stop position. Understood, I'll review this patch. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Thu, Aug 10, 2017 at 9:45 AM, Masahiko Sawada wrote: > Thank you for the patch. Regarding to creating the backup history file > on stanbys, is there any difference from the patch posted on earlier > thread? That's a rebased version on top of what has been applied, except that I have fixed an incorrect comment and shuffled the code building the WAL segment name for the stop position. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating backup history files for backups taken from standbys
On Thu, Aug 10, 2017 at 3:56 PM, Michael Paquier wrote: > Hi all, > > In the recent thread related to a bug in pg_stop_backup when waiting > for WAL segments to be archived, it has been mentioned that it would > be nice to get backup history files generated as well on standbys: > https://www.postgresql.org/message-id/CA+Tgmobh3=quCzugFP5yA3-z5uzz=YKZ7arnoApJoPN7=0h...@mail.gmail.com > > Backup history files are not essential to backups, but they are for > debugging purposes, and it has bugged me hard that we should have a > consistent behavior in this area, particularly because with > archive_mode = always they can be archived by a standby. > > The previous thread has ended with 52f8a59d, which addressed the > original problem but discarded the backup history file generation > during recovery. Attached is a patch to fix this inconsistency, parked > in next CF. I am not arguing for this change as a bug fix, but as an > improvement for PG11. > > Thoughts? Thank you for the patch. Regarding to creating the backup history file on stanbys, is there any difference from the patch posted on earlier thread? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Creating backup history files for backups taken from standbys
Hi all, In the recent thread related to a bug in pg_stop_backup when waiting for WAL segments to be archived, it has been mentioned that it would be nice to get backup history files generated as well on standbys: https://www.postgresql.org/message-id/CA+Tgmobh3=quCzugFP5yA3-z5uzz=YKZ7arnoApJoPN7=0h...@mail.gmail.com Backup history files are not essential to backups, but they are for debugging purposes, and it has bugged me hard that we should have a consistent behavior in this area, particularly because with archive_mode = always they can be archived by a standby. The previous thread has ended with 52f8a59d, which addressed the original problem but discarded the backup history file generation during recovery. Attached is a patch to fix this inconsistency, parked in next CF. I am not arguing for this change as a bug fix, but as an improvement for PG11. Thoughts? -- Michael standby-backup-history.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
