Resolving pg_standby -l

Started by Simon Riggsover 16 years ago5 messages
#1Simon Riggs
simon@2ndQuadrant.com
1 attachment(s)

Short patch to
1. disable pg_standby -l
One line change only appropriate for this stage of release
2. Remove mention of -l and link from docs

pg_standby -l is still accepted, just does nothing (for now).

Existing code maintained in case we backpatch a fix for linking problem
at a later date.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Attachments:

pg_standby_minus_l_fix.v1.patchtext/x-patch; charset=UTF-8; name=pg_standby_minus_l_fix.v1.patchDownload
Index: contrib/pg_standby/pg_standby.c
===================================================================
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.24
diff -c -r1.24 pg_standby.c
*** contrib/pg_standby/pg_standby.c	18 Jun 2009 10:08:08 -0000	1.24
--- contrib/pg_standby/pg_standby.c	25 Jun 2009 10:37:00 -0000
***************
*** 523,529 ****
  	printf("  -d                 generate lots of debugging output (testing only)\n");
  	printf("  -k NUMFILESTOKEEP  if RESTARTWALFILE not used, removes files prior to limit\n"
  		   "                     (0 keeps all)\n");
! 	printf("  -l                 links into archive (leaves file in archive)\n");
  	printf("  -r MAXRETRIES      max number of times to retry, with progressive wait\n"
  		   "                     (default=3)\n");
  	printf("  -s SLEEPTIME       seconds to wait between file checks (min=1, max=60,\n"
--- 523,529 ----
  	printf("  -d                 generate lots of debugging output (testing only)\n");
  	printf("  -k NUMFILESTOKEEP  if RESTARTWALFILE not used, removes files prior to limit\n"
  		   "                     (0 keeps all)\n");
! 	printf("  -l                 does nothing; use of link is now deprecated\n");
  	printf("  -r MAXRETRIES      max number of times to retry, with progressive wait\n"
  		   "                     (default=3)\n");
  	printf("  -s SLEEPTIME       seconds to wait between file checks (min=1, max=60,\n"
***************
*** 610,616 ****
  				}
  				break;
  			case 'l':			/* Use link */
! 				restoreCommandType = RESTORE_COMMAND_LINK;
  				break;
  			case 'r':			/* Retries */
  				maxretries = atoi(optarg);
--- 610,621 ----
  				}
  				break;
  			case 'l':			/* Use link */
! 				/*
! 				 * Link feature disabled, possibly permanently. Linking
! 				 * causes a problem after recovery ends that is not currently
! 				 * resolved by PostgreSQL. 25 Jun 2009
! 					restoreCommandType = RESTORE_COMMAND_LINK;
! 				*/
  				break;
  			case 'r':			/* Retries */
  				maxretries = atoi(optarg);
Index: doc/src/sgml/pgstandby.sgml
===================================================================
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/doc/src/sgml/pgstandby.sgml,v
retrieving revision 2.9
diff -c -r2.9 pgstandby.sgml
*** doc/src/sgml/pgstandby.sgml	14 May 2009 21:59:22 -0000	2.9
--- doc/src/sgml/pgstandby.sgml	25 Jun 2009 10:32:54 -0000
***************
*** 27,33 ****
   <itemizedlist>
    <listitem>
     <para>
!     Supports copy or link for restoring WAL files
     </para>
    </listitem>
    <listitem>
--- 27,33 ----
   <itemizedlist>
    <listitem>
     <para>
!     Uses copy to restore WAL files
     </para>
    </listitem>
    <listitem>
***************
*** 172,193 ****
        </entry>
       </row>
       <row>
-       <entry><literal>-l</></entry>
-       <entry>no</entry>
-       <entry>
-        Use <literal>ln</> command to restore WAL files from archive.
-        Link is more efficient than copy, but the default is copy since link
-        will not work in all scenarios.
-        On Windows, this option uses the <literal>mklink</> command
-        to provide a file-to-file symbolic link. <literal>-l</> will
-        not work on versions of Windows prior to Vista.
-       </entry>
-      </row>
-      <row>
        <entry><literal>-r</> <replaceable>maxretries</></entry>
        <entry>3</entry>
        <entry>
!         Set the maximum number of times to retry the copy or link command if it
          fails. After each failure, we wait for <replaceable>sleeptime</> *
          <replaceable>num_retries</>
          so that the wait time increases progressively.  So by default,
--- 172,181 ----
        </entry>
       </row>
       <row>
        <entry><literal>-r</> <replaceable>maxretries</></entry>
        <entry>3</entry>
        <entry>
!         Set the maximum number of times to retry the copy command if it
          fails. After each failure, we wait for <replaceable>sleeptime</> *
          <replaceable>num_retries</>
          so that the wait time increases progressively.  So by default,
***************
*** 242,248 ****
    <programlisting>
  archive_command = 'cp %p .../archive/%f'
  
! restore_command = 'pg_standby -l -d -s 2 -t /tmp/pgsql.trigger.5442 .../archive %f %p %r 2>>standby.log'
  
  recovery_end_command = 'rm -f /tmp/pgsql.trigger.5442'
    </programlisting>
--- 230,236 ----
    <programlisting>
  archive_command = 'cp %p .../archive/%f'
  
! restore_command = 'pg_standby -d -s 2 -t /tmp/pgsql.trigger.5442 .../archive %f %p %r 2>>standby.log'
  
  recovery_end_command = 'rm -f /tmp/pgsql.trigger.5442'
    </programlisting>
***************
*** 255,265 ****
    <itemizedlist>
     <listitem>
      <para>
-      use the <literal>ln</> command to restore WAL files from archive
-     </para>
-    </listitem>
-    <listitem>
-     <para>
       produce debugging output in <filename>standby.log</>
      </para>
     </listitem>
--- 243,248 ----
#2Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Simon Riggs (#1)
Re: Resolving pg_standby -l

Simon Riggs wrote:

Short patch to
1. disable pg_standby -l
One line change only appropriate for this stage of release
2. Remove mention of -l and link from docs

pg_standby -l is still accepted, just does nothing (for now).

Existing code maintained in case we backpatch a fix for linking problem
at a later date.

Ah, I had forgotten about this already.

Committed. The patch looks very safe to me, but given that we're just
about to wrap the release I'm keeping my fingers crossed that this
didn't break anything,.

I didn't commit this to the back-branches yet, because I'm not sure if
we have consensus on that. If symlinking has a meaningful performance
advantage, someone might be unhappy if we disable that option in a minor
release. I think we should go ahead anyway, but does anyone object?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#1)
Re: Resolving pg_standby -l

Simon Riggs <simon@2ndQuadrant.com> writes:

--- 610,621 ----
}
break;
case 'l':			/* Use link */
! 				/*
! 				 * Link feature disabled, possibly permanently. Linking
! 				 * causes a problem after recovery ends that is not currently
! 				 * resolved by PostgreSQL. 25 Jun 2009
! 					restoreCommandType = RESTORE_COMMAND_LINK;
! 				*/
break;
case 'r':			/* Retries */
maxretries = atoi(optarg);

Just for future reference: the above is going to look like hell after
pgindent gets its hands on it. Better style for this project is

/*
* ordinary comment block
*/
#ifdef NOT_USED
code to be disabled
#endif

regards, tom lane

#4Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#3)
1 attachment(s)
Re: Resolving pg_standby -l

On Thu, 2009-06-25 at 09:51 -0400, Tom Lane wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

--- 610,621 ----
}
break;
case 'l':			/* Use link */
! 				/*
! 				 * Link feature disabled, possibly permanently. Linking
! 				 * causes a problem after recovery ends that is not currently
! 				 * resolved by PostgreSQL. 25 Jun 2009
! 					restoreCommandType = RESTORE_COMMAND_LINK;
! 				*/
break;
case 'r':			/* Retries */
maxretries = atoi(optarg);

Just for future reference: the above is going to look like hell after
pgindent gets its hands on it. Better style for this project is

/*
* ordinary comment block
*/
#ifdef NOT_USED
code to be disabled
#endif

Will do, thanks. Patch to cleanup as advised is attached.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Attachments:

pg_standby_code_clean.patchtext/x-patch; charset=utf-8; name=pg_standby_code_clean.patchDownload
Index: contrib/pg_standby/pg_standby.c
===================================================================
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.25
diff -c -r1.25 pg_standby.c
*** contrib/pg_standby/pg_standby.c	25 Jun 2009 12:03:10 -0000	1.25
--- contrib/pg_standby/pg_standby.c	25 Jun 2009 15:12:27 -0000
***************
*** 614,621 ****
  				 * Link feature disabled, possibly permanently. Linking
  				 * causes a problem after recovery ends that is not currently
  				 * resolved by PostgreSQL. 25 Jun 2009
! 					restoreCommandType = RESTORE_COMMAND_LINK;
! 				*/
  				break;
  			case 'r':			/* Retries */
  				maxretries = atoi(optarg);
--- 614,623 ----
  				 * Link feature disabled, possibly permanently. Linking
  				 * causes a problem after recovery ends that is not currently
  				 * resolved by PostgreSQL. 25 Jun 2009
! 				 */
! #ifdef NOT_USED
! 				restoreCommandType = RESTORE_COMMAND_LINK;
! #endif
  				break;
  			case 'r':			/* Retries */
  				maxretries = atoi(optarg);
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#4)
Re: Resolving pg_standby -l

Simon Riggs <simon@2ndQuadrant.com> writes:

Will do, thanks. Patch to cleanup as advised is attached.

Applied, thanks.

regards, tom lane