Thread: Re: [HACKERS] Minor changes to Recovery related code

Re: [HACKERS] Minor changes to Recovery related code

From
Simon Riggs
Date:
On Thu, 2008-03-27 at 17:34 +0000, Simon Riggs wrote:
> Follow-up during March 2008 CommitFest
>
> On Thu, 2007-06-07 at 21:53 +0100, Simon Riggs wrote:
> > On Sat, 2007-03-31 at 00:51 +0200, Florian G. Pflug wrote:

> > - pg_stop_backup() will wait until the WAL file that ends the backup is
> > safely archived, even if a failure to archive occurs. This is a change
> > to current behaviour, but since it implements the originally *expected*
> > behaviour IMHO it should be the default.
>
> The most straightforward thing is to make pg_stop_backup() wait as
> described above.
>
> If people want it to timeout, they can use a statement_timeout as
> suggested by Florian.
>
> This can be implemented by having the function poll in an infinite loop
> for archive_status/LOG.done. Also, as Florian suggests, we should have
> it output a WARNING message every 60 seconds.
>
> I'll write a patch now.

Patch to implement waiting pg_stop_backup().

Tested and ready to apply, includes docs.

--
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com

  PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk

Attachment

Re: [HACKERS] Minor changes to Recovery related code

From
Bruce Momjian
Date:
Nice, applied.  I only modified some of the documentation wording.

I was a little worried that statement_timeout might cancel
pg_stop_backup() _before_ it had gotten to waiting for the archive logs
to be saved, but based on what little code there is before that block, I
think we are OK.

---------------------------------------------------------------------------

Simon Riggs wrote:
> On Thu, 2008-03-27 at 17:34 +0000, Simon Riggs wrote:
> > Follow-up during March 2008 CommitFest
> >
> > On Thu, 2007-06-07 at 21:53 +0100, Simon Riggs wrote:
> > > On Sat, 2007-03-31 at 00:51 +0200, Florian G. Pflug wrote:
>
> > > - pg_stop_backup() will wait until the WAL file that ends the backup is
> > > safely archived, even if a failure to archive occurs. This is a change
> > > to current behaviour, but since it implements the originally *expected*
> > > behaviour IMHO it should be the default.
> >
> > The most straightforward thing is to make pg_stop_backup() wait as
> > described above.
> >
> > If people want it to timeout, they can use a statement_timeout as
> > suggested by Florian.
> >
> > This can be implemented by having the function poll in an infinite loop
> > for archive_status/LOG.done. Also, as Florian suggests, we should have
> > it output a WARNING message every 60 seconds.
> >
> > I'll write a patch now.
>
> Patch to implement waiting pg_stop_backup().
>
> Tested and ready to apply, includes docs.
>
> --
>   Simon Riggs
>   2ndQuadrant  http://www.2ndQuadrant.com
>
>   PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk

[ Attachment, skipping... ]

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/backup.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/backup.sgml,v
retrieving revision 2.116
diff -c -c -r2.116 backup.sgml
*** doc/src/sgml/backup.sgml    28 Mar 2008 15:00:28 -0000    2.116
--- doc/src/sgml/backup.sgml    5 Apr 2008 01:28:09 -0000
***************
*** 761,772 ****
      <para>
       Once the WAL segment files used during the backup are archived, you are
       done.  The file identified by <function>pg_stop_backup</>'s result is
!      the last segment that needs to be archived to complete the backup.
!      Archival of these files will happen automatically, since you have
!      already configured <varname>archive_command</>. In many cases, this
!      happens fairly quickly, but you are advised to monitor your archival
!      system to ensure this has taken place so that you can be certain you
!      have a complete backup.
      </para>
     </listitem>
    </orderedlist>
--- 761,779 ----
      <para>
       Once the WAL segment files used during the backup are archived, you are
       done.  The file identified by <function>pg_stop_backup</>'s result is
!      the last segment that is required to form a complete set of backup files.
!      <function>pg_stop_backup</> does not return until the last segment has
!      been archived.
!      Archiving of these files happens automatically since you have
!      already configured <varname>archive_command</>. In most cases this
!      happens quickly, but you are advised to monitor your archive
!      system to ensure there are no delays.
!      If the archive process has fallen behind
!      because of failures of the archive command, it will keep retrying
!      until the archive succeeds and the backup is complete.
!      If you wish to place a time limit on the execution of
!      <function>pg_stop_backup</>, set an appropriate
!      <varname>statement_timeout</varname> value.
      </para>
     </listitem>
    </orderedlist>
***************
*** 1044,1050 ****
     <note>
       <para>
        The stop point must be after the ending time of the base backup, i.e.,
!       the time of <function>pg_stop_backup</>.  You cannot use a base backup
        to recover to a time when that backup was still going on.  (To
        recover to such a time, you must go back to your previous base backup
        and roll forward from there.)
--- 1051,1057 ----
     <note>
       <para>
        The stop point must be after the ending time of the base backup, i.e.,
!       the end time of <function>pg_stop_backup</>.  You cannot use a base backup
        to recover to a time when that backup was still going on.  (To
        recover to such a time, you must go back to your previous base backup
        and roll forward from there.)
***************
*** 1322,1327 ****
--- 1329,1335 ----
        After the backup the switch file is removed. Archived WAL files are
        then added to the backup so that both base backup and all required
        WAL files are part of the same <application>tar</> file.
+       Please remember to add error handling to your backup scripts.
       </para>
      </sect3>

Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.295
diff -c -c -r1.295 xlog.c
*** src/backend/access/transam/xlog.c    25 Mar 2008 22:42:42 -0000    1.295
--- src/backend/access/transam/xlog.c    5 Apr 2008 01:28:09 -0000
***************
*** 382,388 ****

  static void XLogArchiveNotify(const char *xlog);
  static void XLogArchiveNotifySeg(uint32 log, uint32 seg);
! static bool XLogArchiveCheckDone(const char *xlog);
  static void XLogArchiveCleanup(const char *xlog);
  static void readRecoveryCommandFile(void);
  static void exitArchiveRecovery(TimeLineID endTLI,
--- 382,388 ----

  static void XLogArchiveNotify(const char *xlog);
  static void XLogArchiveNotifySeg(uint32 log, uint32 seg);
! static bool XLogArchiveCheckDone(const char *xlog, bool create_if_missing);
  static void XLogArchiveCleanup(const char *xlog);
  static void readRecoveryCommandFile(void);
  static void exitArchiveRecovery(TimeLineID endTLI,
***************
*** 1128,1134 ****
   * create <XLOG>.ready fails, we'll retry during subsequent checkpoints.
   */
  static bool
! XLogArchiveCheckDone(const char *xlog)
  {
      char        archiveStatusPath[MAXPGPATH];
      struct stat stat_buf;
--- 1128,1134 ----
   * create <XLOG>.ready fails, we'll retry during subsequent checkpoints.
   */
  static bool
! XLogArchiveCheckDone(const char *xlog, bool create_if_missing)
  {
      char        archiveStatusPath[MAXPGPATH];
      struct stat stat_buf;
***************
*** 1153,1159 ****
          return true;

      /* Retry creation of the .ready file */
!     XLogArchiveNotify(xlog);
      return false;
  }

--- 1153,1161 ----
          return true;

      /* Retry creation of the .ready file */
!     if (create_if_missing)
!         XLogArchiveNotify(xlog);
!
      return false;
  }

***************
*** 2704,2710 ****
              strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
              strcmp(xlde->d_name + 8, lastoff + 8) <= 0)
          {
!             if (XLogArchiveCheckDone(xlde->d_name))
              {
                  snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlde->d_name);

--- 2706,2712 ----
              strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
              strcmp(xlde->d_name + 8, lastoff + 8) <= 0)
          {
!             if (XLogArchiveCheckDone(xlde->d_name, true))
              {
                  snprintf(path, MAXPGPATH, XLOGDIR "/%s", xlde->d_name);

***************
*** 2771,2777 ****
              strcmp(xlde->d_name + strlen(xlde->d_name) - strlen(".backup"),
                     ".backup") == 0)
          {
!             if (XLogArchiveCheckDone(xlde->d_name))
              {
                  ereport(DEBUG2,
                  (errmsg("removing transaction log backup history file \"%s\"",
--- 2773,2779 ----
              strcmp(xlde->d_name + strlen(xlde->d_name) - strlen(".backup"),
                     ".backup") == 0)
          {
!             if (XLogArchiveCheckDone(xlde->d_name, true))
              {
                  ereport(DEBUG2,
                  (errmsg("removing transaction log backup history file \"%s\"",
***************
*** 6556,6561 ****
--- 6558,6565 ----
      FILE       *fp;
      char        ch;
      int            ich;
+     int            seconds_before_warning;
+     int            waits = 0;

      if (!superuser())
          ereport(ERROR,
***************
*** 6660,6665 ****
--- 6664,6702 ----
      CleanupBackupHistory();

      /*
+      * Wait until the history file has been archived. We assume that the
+      * alphabetic sorting property of the WAL files ensures the last WAL
+      * file is guaranteed archived by the time the history file is archived.
+      *
+      * We wait forever, since archive_command is supposed to work and
+      * we assume the admin wanted his backup to work completely. If you
+      * don't wish to wait, you can SET statement_timeout = xx;
+      *
+      * If the status file is missing, we assume that is because it was
+      * set to .ready before we slept, then while asleep it has been set
+      * to .done and then removed by a concurrent checkpoint.
+      */
+     BackupHistoryFileName(histfilepath, ThisTimeLineID, _logId, _logSeg,
+                           startpoint.xrecoff % XLogSegSize);
+
+     seconds_before_warning = 60;
+     waits = 0;
+
+     while (!XLogArchiveCheckDone(histfilepath, false))
+     {
+         CHECK_FOR_INTERRUPTS();
+
+         pg_usleep(1000000L);
+
+         if (++waits >= seconds_before_warning)
+         {
+             seconds_before_warning *= 2;     /* This wraps in >10 years... */
+             elog(WARNING, "pg_stop_backup() waiting for archive to complete "
+                             "(%d seconds delay)", waits);
+         }
+     }
+
+     /*
       * We're done.  As a convenience, return the ending WAL location.
       */
      snprintf(stopxlogfilename, sizeof(stopxlogfilename), "%X/%X",