Re: gcc 4.6 and hot standby - Mailing list pgsql-hackers

Alex Hunsaker <badalex@gmail.com> writes:
> On Fri, Jun 10, 2011 at 12:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think we need a workaround.

My second idea about moving the test up doesn't work, because we can't
know the page header size until after we've read the page.  But I've
verified that the attached patch does make the problem go away on my
F15 box.

> Hrm, Couldn't we change all the references to tmpRecPtr to use RecPtr
> instead? (Except of course where we assign RecPtr = &tmpRecPtr); I
> think that would make the code look a lot less confused. Something
> like the attached?

Yeah, we could do that too; slightly modified version of your change
included in the attached.

            regards, tom lane

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5c3ca479fb33fff646e3a7b08b53efea92b9a97f..aa0b0291ee1c7781a36c62e3d89abbc98d3b8499 100644
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*************** ReadRecord(XLogRecPtr *RecPtr, int emode
*** 3728,3750 ****
          RecPtr = &tmpRecPtr;

          /*
!          * Align recptr to next page if no more records can fit on the current
!          * page.
           */
          if (XLOG_BLCKSZ - (RecPtr->xrecoff % XLOG_BLCKSZ) < SizeOfXLogRecord)
!         {
!             NextLogPage(tmpRecPtr);
!             /* We will account for page header size below */
!         }

!         if (tmpRecPtr.xrecoff >= XLogFileSize)
          {
!             (tmpRecPtr.xlogid)++;
!             tmpRecPtr.xrecoff = 0;
          }
      }
      else
      {
          if (!XRecOffIsValid(RecPtr->xrecoff))
              ereport(PANIC,
                      (errmsg("invalid record offset at %X/%X",
--- 3728,3759 ----
          RecPtr = &tmpRecPtr;

          /*
!          * RecPtr is pointing to end+1 of the previous WAL record.  We must
!          * advance it if necessary to where the next record starts.  First,
!          * align to next page if no more records can fit on the current page.
           */
          if (XLOG_BLCKSZ - (RecPtr->xrecoff % XLOG_BLCKSZ) < SizeOfXLogRecord)
!             NextLogPage(*RecPtr);

!         /* Check for crossing of xlog segment boundary */
!         if (RecPtr->xrecoff >= XLogFileSize)
          {
!             (RecPtr->xlogid)++;
!             RecPtr->xrecoff = 0;
          }
+
+         /*
+          * If at page start, we must skip over the page header.  But we can't
+          * do that until we've read in the page, since the header size is
+          * variable.
+          */
      }
      else
      {
+         /*
+          * In this case, the passed-in record pointer should already be
+          * pointing to a valid record starting position.
+          */
          if (!XRecOffIsValid(RecPtr->xrecoff))
              ereport(PANIC,
                      (errmsg("invalid record offset at %X/%X",
*************** retry:
*** 3773,3783 ****
      if (targetRecOff == 0)
      {
          /*
!          * Can only get here in the continuing-from-prev-page case, because
!          * XRecOffIsValid eliminated the zero-page-offset case otherwise. Need
!          * to skip over the new page's header.
           */
!         tmpRecPtr.xrecoff += pageHeaderSize;
          targetRecOff = pageHeaderSize;
      }
      else if (targetRecOff < pageHeaderSize)
--- 3782,3794 ----
      if (targetRecOff == 0)
      {
          /*
!          * At page start, so skip over page header.  The Assert checks that
!          * we're not scribbling on caller's record pointer; it's OK because we
!          * can only get here in the continuing-from-prev-record case, since
!          * XRecOffIsValid rejected the zero-page-offset case otherwise.
           */
!         Assert(RecPtr == &tmpRecPtr);
!         RecPtr->xrecoff += pageHeaderSize;
          targetRecOff = pageHeaderSize;
      }
      else if (targetRecOff < pageHeaderSize)
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index eeccdce31d076322cc5431117c6705b839b1b162..7e39630c1bf5d7cbf1a721b641a9481069e92816 100644
*** a/src/include/access/xlog_internal.h
--- b/src/include/access/xlog_internal.h
*************** typedef XLogLongPageHeaderData *XLogLong
*** 154,166 ****
  /* Align a record pointer to next page */
  #define NextLogPage(recptr) \
      do {    \
!         if (recptr.xrecoff % XLOG_BLCKSZ != 0)    \
!             recptr.xrecoff +=    \
!                 (XLOG_BLCKSZ - recptr.xrecoff % XLOG_BLCKSZ);    \
!         if (recptr.xrecoff >= XLogFileSize) \
          {    \
!             (recptr.xlogid)++;    \
!             recptr.xrecoff = 0; \
          }    \
      } while (0)

--- 154,166 ----
  /* Align a record pointer to next page */
  #define NextLogPage(recptr) \
      do {    \
!         if ((recptr).xrecoff % XLOG_BLCKSZ != 0)    \
!             (recptr).xrecoff +=    \
!                 (XLOG_BLCKSZ - (recptr).xrecoff % XLOG_BLCKSZ);    \
!         if ((recptr).xrecoff >= XLogFileSize) \
          {    \
!             ((recptr).xlogid)++;    \
!             (recptr).xrecoff = 0; \
          }    \
      } while (0)