On Fri, Jun 10, 2011 at 12:38, Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> I've been able to reproduce this on released Fedora 15, and sure enough
> it is a compiler bug. The problem comes from these fragments of
> ReadRecord():
> [ ... ]
Whoa, awesome. I spent a few more hours comparing the assembly-- then
I tried compiling a subset of xlog.c with some educated guesses as to
what function might be getting mis-compiled. Obviously my guesses were
not educated enough! :-)
> I've filed a bug report for this:
> https://bugzilla.redhat.com/show_bug.cgi?id=712480
> but I wouldn't care to hold my breath while waiting for a fix to appear
> upstream, let alone propagate to all the distros already using 4.6.0.
I wouldn't hold my breath either.
> I think we need a workaround.
>
> The obvious question to ask here is why are we updating
> "tmpRecPtr.xrecoff" and not "RecPtr->xrecoff"? The latter choice would
> be a lot more readable, since the immediately surrounding code doesn't
> refer to tmpRecPtr. I think the idea is to guarantee that the caller's
> referenced record pointer (if any) isn't modified, but if RecPtr is not
> pointing at tmpRecPtr here, we have got big problems anyway.
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?
FYI Im happy to test whatever you fix you propose for a few days on
this machine. It gets a fair amount of traffic... hopefully enough to
exercise some possible corner cases.
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 3681,3694 **** ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
*/
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
--- 3681,3694 ----
*/
if (XLOG_BLCKSZ - (RecPtr->xrecoff % XLOG_BLCKSZ) < SizeOfXLogRecord)
{
! NextLogPage(RecPtr);
/* We will account for page header size below */
}
! if (RecPtr->xrecoff >= XLogFileSize)
{
! (RecPtr->xlogid)++;
! RecPtr->xrecoff = 0;
}
}
else
***************
*** 3725,3731 **** retry:
* 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)
--- 3725,3731 ----
* XRecOffIsValid eliminated the zero-page-offset case otherwise. Need
* to skip over the new page's header.
*/
! RecPtr->xrecoff += pageHeaderSize;
targetRecOff = pageHeaderSize;
}
else if (targetRecOff < pageHeaderSize)
*** a/src/include/access/xlog_internal.h
--- b/src/include/access/xlog_internal.h
***************
*** 154,166 **** typedef XLogLongPageHeaderData *XLogLongPageHeader;
/* 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)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
