Re: Small fixes about backup history file in doc and pg_standby

2018-07-01 Thread Peter Eisentraut
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

2018-06-27 Thread Yugo Nagata
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

2018-06-27 Thread Michael Paquier
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

2018-06-27 Thread Yugo Nagata
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

2018-06-27 Thread Yugo Nagata
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

2018-06-26 Thread Fujii Masao
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

2018-06-26 Thread Kyotaro HORIGUCHI
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

2018-06-26 Thread Yugo Nagata
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