Confusing comment in xlog.c or am I missing something?
I was just going through the xlog.c and I came across following which
confused me:
Given,
src/include/access/xlogdefs.h
#define XLogSegsPerFile (((uint32) 0xffffffff) / XLogSegSize)
#define XLogFileSize (XLogSegsPerFile * XLogSegSize)
Also,
typedef struct XLogRecPtr
{
uint32 xlogid; /* log file #, 0 based */
uint32 xrecoff; /* byte offset of location
in log file */
} XLogRecPtr;
Check for crossing of xlog "segment" boundary?
src/include/access/trasam/xlog.c
/* Check for crossing of xlog segment boundary */
if (RecPtr->xrecoff >= XLogFileSize)
{
(RecPtr->xlogid)++;
RecPtr->xrecoff = 0;
}
Is that xlog "file" boundary or am I missing something?
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Confusing-comment-in-xlog-c-or-am-I-missing-something-tp5754010.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02.05.2013 08:16, Amit Langote wrote:
I was just going through the xlog.c and I came across following which
confused me:Given,
src/include/access/xlogdefs.h
#define XLogSegsPerFile (((uint32) 0xffffffff) / XLogSegSize)
#define XLogFileSize (XLogSegsPerFile * XLogSegSize)Also,
typedef struct XLogRecPtr
{
uint32 xlogid; /* log file #, 0 based */
uint32 xrecoff; /* byte offset of location
in log file */
} XLogRecPtr;Check for crossing of xlog "segment" boundary?
src/include/access/trasam/xlog.c
/* Check for crossing of xlog segment boundary */
if (RecPtr->xrecoff>= XLogFileSize)
{
(RecPtr->xlogid)++;
RecPtr->xrecoff = 0;
}Is that xlog "file" boundary or am I missing something?
The WAL space is divided into 4GB "log files", which are not physical
files but purely logical constructs. The XLogRecPtr.xlogid field
indicates the logical log file, and xrecoff is the offset within the
file. Each logical log file is divided into "log segments", which are
the physical files you see in pg_xlog. See also the comment above the
definition of XLogRecPtr.
This split was necessary to deal with more than 4 GB of WAL, using only
32-bit integers. In 9.3, that's changed, and XLogRecPtr is a 64-bit
integer. The whole concept of "logical log files" is gone, so it's a lot
less confusing now.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
/* Check for crossing of xlog segment boundary */
if (RecPtr->xrecoff>= XLogFileSize)
{
(RecPtr->xlogid)++;
RecPtr->xrecoff = 0;
}Is that xlog "file" boundary or am I missing something?
The WAL space is divided into 4GB "log files", which are not physical
files but purely logical constructs. The XLogRecPtr.xlogid field
indicates the logical log file, and xrecoff is the offset within the
file. Each logical log file is divided into "log segments", which are
the physical files you see in pg_xlog. See also the comment above the
definition of XLogRecPtr.This split was necessary to deal with more than 4 GB of WAL, using only
32-bit integers. In 9.3, that's changed, and XLogRecPtr is a 64-bit
integer. The whole concept of "logical log files" is gone, so it's a lot
less confusing now.- Heikki
I see. However, although in 9.3 XLogRecPtr is a 64bit integer, in 9.2
branch, it is
still a struct with xlogid and xrecoff (both uint32) as its members.
In that case, should the comment be "/* Check for crossing of xlog
file boundary */"
instead of /* Check for crossing of xlog segment boundary */, since
( RecPtr->xrecoff >= XLogFileSize )
would mean crossing the xlog "file" (not segment) boundary, right?
Am I still missing something as in 9.2?
Amit Langote
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Confusing-comment-in-xlog-c-or-am-I-missing-something-tp5754010p5754014.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
On 02.05.2013 09:11, Amit Langote wrote:
In that case, should the comment be "/* Check for crossing of xlog
file boundary */"
instead of /* Check for crossing of xlog segment boundary */, since
( RecPtr->xrecoff>= XLogFileSize )
would mean crossing the xlog "file" (not segment) boundary, right?
Yeah, that would be more correct. The phrase we seem to use elsewhere in
xlog.c is "crossing a logid boundary".
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Yeah, that would be more correct. The phrase we seem to use elsewhere in
xlog.c is "crossing a logid boundary".
Should we change it in 9.2 to clear the confusion?
(Attached is a rather small patch to fix that! :) )
--
Amit Langote
minor-xlog-comment.patch (914 bytes) <http://postgresql.1045698.n5.nabble.com/attachment/5754017/0/minor-xlog-comment.patch>
--
View this message in context: http://postgresql.1045698.n5.nabble.com/Confusing-comment-in-xlog-c-or-am-I-missing-something-tp5754010p5754017.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Attached patch fixes this.
--
Amit Langote
Attachments:
minor-xlog-comment-fix.patchapplication/octet-stream; name=minor-xlog-comment-fix.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 26c9ef3..d4f9696 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3970,7 +3970,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
if (XLOG_BLCKSZ - (RecPtr->xrecoff % XLOG_BLCKSZ) < SizeOfXLogRecord)
NextLogPage(*RecPtr);
- /* Check for crossing of xlog segment boundary */
+ /* Check for crossing of xlog logid boundary */
if (RecPtr->xrecoff >= XLogFileSize)
{
(RecPtr->xlogid)++;Import Notes
Resolved by subject fallback
On 02.05.2013 18:05, Amit Langote wrote:
Attached patch fixes this.
ok, applied.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers