Re: Small fixes about backup history file in doc and pg_standby
On 27.06.18 18:22, Yugo Nagata wrote: > On Wed, 27 Jun 2018 18:36:46 +0900 > Michael Paquier wrote: > >> On Wed, Jun 27, 2018 at 05:42:07PM +0900, Yugo Nagata wrote: >>> On Wed, 27 Jun 2018 00:58:18 +0900 >>> Fujii Masao wrote: > In addition, the current pg_standby still can handle a backup history > file that are > never requested. It is harmless but unnecessary code. Another attached > patch > (pg_standby.patch) removes this part of code. Since this is not bug fix, let's discuss this in 12dev cycle. >> >> +1. > > I added this to CF. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Small fixes about backup history file in doc and pg_standby
On Wed, 27 Jun 2018 18:36:46 +0900 Michael Paquier wrote: > On Wed, Jun 27, 2018 at 05:42:07PM +0900, Yugo Nagata wrote: > > On Wed, 27 Jun 2018 00:58:18 +0900 > > Fujii Masao wrote: > >>> In addition, the current pg_standby still can handle a backup history > >>> file that are > >>> never requested. It is harmless but unnecessary code. Another attached > >>> patch > >>> (pg_standby.patch) removes this part of code. > >> > >> Since this is not bug fix, let's discuss this in 12dev cycle. > > +1. I added this to CF. > -- > Michael -- Yugo Nagata
Re: Small fixes about backup history file in doc and pg_standby
On Wed, Jun 27, 2018 at 05:42:07PM +0900, Yugo Nagata wrote: > On Wed, 27 Jun 2018 00:58:18 +0900 > Fujii Masao wrote: >>> In addition, the current pg_standby still can handle a backup history file >>> that are >>> never requested. It is harmless but unnecessary code. Another attached patch >>> (pg_standby.patch) removes this part of code. >> >> Since this is not bug fix, let's discuss this in 12dev cycle. +1. -- Michael signature.asc Description: PGP signature
Re: Small fixes about backup history file in doc and pg_standby
On Tue, 26 Jun 2018 20:19:42 +0900 (Tokyo Standard Time) Kyotaro HORIGUCHI wrote: > Hello. > > Good catch! > > At Tue, 26 Jun 2018 17:47:52 +0900, Yugo Nagata wrote > in <20180626174752.0ce505e3.nag...@sraoss.co.jp> > > Hi, > > > > While looking into the backup and recovery code, I found small > > documentation bugs. > > The documatation says that the backup history files can be requested for > > recovery, > > but it's not used by the system and not requested anymore since PG 9.0 > > (commit 06f82b29616cd9effcaefd99c6b6e2e80697482f) and never be requested. > > > > Attached patch (doc_backup_history_file.patch) corrects the description > > about this. > > > > In addition, the current pg_standby still can handle a backup history file > > that are > > never requested. It is harmless but unnecessary code. Another attached patch > > (pg_standby.patch) removes this part of code. > > The comment fix seems fine and they seem to be all occurances of > the word ".backup" in the context of recovery_command. > > The definition of the symbol XLOG_BACKUP_LABEL is no longer > useful after your patch applied. Removing the symbol makes > XLOG_DATA and the variable nextWALFileName useless and finally we > can remove all branching using it. Thank you for your reviewing my patch. I've also removed XLOG_BACKUP_LABEL, but I left nextWALFileName since this is still referred in CustomizableCleanupPriorWALFiles(). Attached is the updated patch. Regards, > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center > > -- Yugo Nagata diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index cb78597..b37cf6d 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -94,7 +94,6 @@ int restoreCommandType; #define XLOG_DATA 0 #define XLOG_HISTORY 1 -#define XLOG_BACKUP_LABEL 2 int nextWALFileType; #define SET_RESTORE_COMMAND(cmd, arg1, arg2) \ @@ -211,15 +210,9 @@ CustomizableNextWALFileReady(void) } /* - * If it's a backup file, return immediately. If it's a regular file * return only if it's the right size already. */ - if (IsBackupHistoryFileName(nextWALFileName)) - { - nextWALFileType = XLOG_BACKUP_LABEL; - return true; - } - else if (WalSegSz > 0 && stat_buf.st_size == WalSegSz) + if (WalSegSz > 0 && stat_buf.st_size == WalSegSz) { #ifdef WIN32
Re: Small fixes about backup history file in doc and pg_standby
On Wed, 27 Jun 2018 00:58:18 +0900 Fujii Masao wrote: > On Tue, Jun 26, 2018 at 5:47 PM, Yugo Nagata wrote: > > Hi, > > > > While looking into the backup and recovery code, I found small > > documentation bugs. > > The documatation says that the backup history files can be requested for > > recovery, > > but it's not used by the system and not requested anymore since PG 9.0 > > (commit 06f82b29616cd9effcaefd99c6b6e2e80697482f) and never be requested. > > > > Attached patch (doc_backup_history_file.patch) corrects the description > > about this. > > Pushed. Thanks! Thanks! > > > In addition, the current pg_standby still can handle a backup history file > > that are > > never requested. It is harmless but unnecessary code. Another attached patch > > (pg_standby.patch) removes this part of code. > > Since this is not bug fix, let's discuss this in 12dev cycle. Certainly. Regards, > > Regards, > > -- > Fujii Masao > -- Yugo Nagata
Re: Small fixes about backup history file in doc and pg_standby
On Tue, Jun 26, 2018 at 5:47 PM, Yugo Nagata wrote: > Hi, > > While looking into the backup and recovery code, I found small documentation > bugs. > The documatation says that the backup history files can be requested for > recovery, > but it's not used by the system and not requested anymore since PG 9.0 > (commit 06f82b29616cd9effcaefd99c6b6e2e80697482f) and never be requested. > > Attached patch (doc_backup_history_file.patch) corrects the description about > this. Pushed. Thanks! > In addition, the current pg_standby still can handle a backup history file > that are > never requested. It is harmless but unnecessary code. Another attached patch > (pg_standby.patch) removes this part of code. Since this is not bug fix, let's discuss this in 12dev cycle. Regards, -- Fujii Masao
Re: Small fixes about backup history file in doc and pg_standby
Hello. Good catch! At Tue, 26 Jun 2018 17:47:52 +0900, Yugo Nagata wrote in <20180626174752.0ce505e3.nag...@sraoss.co.jp> > Hi, > > While looking into the backup and recovery code, I found small documentation > bugs. > The documatation says that the backup history files can be requested for > recovery, > but it's not used by the system and not requested anymore since PG 9.0 > (commit 06f82b29616cd9effcaefd99c6b6e2e80697482f) and never be requested. > > Attached patch (doc_backup_history_file.patch) corrects the description about > this. > > In addition, the current pg_standby still can handle a backup history file > that are > never requested. It is harmless but unnecessary code. Another attached patch > (pg_standby.patch) removes this part of code. The comment fix seems fine and they seem to be all occurances of the word ".backup" in the context of recovery_command. The definition of the symbol XLOG_BACKUP_LABEL is no longer useful after your patch applied. Removing the symbol makes XLOG_DATA and the variable nextWALFileName useless and finally we can remove all branching using it. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Small fixes about backup history file in doc and pg_standby
Hi, While looking into the backup and recovery code, I found small documentation bugs. The documatation says that the backup history files can be requested for recovery, but it's not used by the system and not requested anymore since PG 9.0 (commit 06f82b29616cd9effcaefd99c6b6e2e80697482f) and never be requested. Attached patch (doc_backup_history_file.patch) corrects the description about this. In addition, the current pg_standby still can handle a backup history file that are never requested. It is harmless but unnecessary code. Another attached patch (pg_standby.patch) removes this part of code. Regards, -- Yugo Nagata diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index cb78597..d957f44 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -211,15 +211,9 @@ CustomizableNextWALFileReady(void) } /* - * If it's a backup file, return immediately. If it's a regular file * return only if it's the right size already. */ - if (IsBackupHistoryFileName(nextWALFileName)) - { - nextWALFileType = XLOG_BACKUP_LABEL; - return true; - } - else if (WalSegSz > 0 && stat_buf.st_size == WalSegSz) + if (WalSegSz > 0 && stat_buf.st_size == WalSegSz) { #ifdef WIN32 diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 982776c..3fa5efd 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1288,7 +1288,7 @@ restore_command = 'cp /mnt/server/archivedir/%f %p' Not all of the requested files will be WAL segment files; you should also expect requests for files with a suffix of -.backup or .history. Also be aware that +.history. Also be aware that the base name of the %p path will be different from %f; do not expect them to be interchangeable. diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index 46bf198..934eb90 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -1524,7 +1524,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)' processing would request a file from the WAL archive, reporting failure if the file was unavailable. For standby processing it is normal for the next WAL file to be unavailable, so the standby must wait for -it to appear. For files ending in .backup or +it to appear. For files ending in .history there is no need to wait, and a non-zero return code must be returned. A waiting restore_command can be written as a custom script that loops after polling for the existence of