Thread: 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.
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
<div class="shrinkable-quote">> > /* Check for crossing of xlog segment boundary */ <br />> > if
(RecPtr->xrecoff>=XLogFileSize) <br />> > { <br />> > (RecPtr->xlogid)++; <br />> >
RecPtr->xrecoff = 0; <br />> > } <br />> > <br />> > Is that xlog "file" boundary or am I
missingsomething? <br />> <br />> The WAL space is divided into 4GB "log files", which are not physical <br
/>>files but purely logical constructs. The XLogRecPtr.xlogid field <br />> indicates the logical log file, and
xrecoffis the offset within the <br />> file. Each logical log file is divided into "log segments", which are <br
/>>the physical files you see in pg_xlog. See also the comment above the <br />> definition of XLogRecPtr. <br
/>><br />> This split was necessary to deal with more than 4 GB of WAL, using only <br />> 32-bit integers. In
9.3,that's changed, and XLogRecPtr is a 64-bit <br />> integer. The whole concept of "logical log files" is gone, so
it'sa lot <br />> less confusing now. <br />> <br />> - Heikki </div><br /><br />I see. However, although in
9.3XLogRecPtr is a 64bit integer, in 9.2 <br />branch, it is <br />still a struct with xlogid and xrecoff (both uint32)
asits members. <br /><br />In that case, should the comment be "/* Check for crossing of xlog <br />file boundary */"
<br/>instead of /* Check for crossing of xlog segment boundary */, since <br />( RecPtr->xrecoff >= XLogFileSize
)<br />would mean crossing the xlog "file" (not segment) boundary, right? <br />Am I still missing something as in 9.2?
<br/><br />Amit Langote <br /><br /><hr align="left" width="300" /> View this message in context: <a
href="http://postgresql.1045698.n5.nabble.com/Confusing-comment-in-xlog-c-or-am-I-missing-something-tp5754010p5754014.html">Re:
Confusingcomment in xlog.c or am I missing something?</a><br /> Sent from the <a
href="http://postgresql.1045698.n5.nabble.com/PostgreSQL-hackers-f1928748.html">PostgreSQL- hackers mailing list
archive</a>at Nabble.com.<br />
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
> Yeah, that would be more correct. The phrase we seem to use elsewhere in <br />> xlog.c is "crossing a logid boundary".<br /><br />Should we change it in 9.2 to clear the confusion? <br /><br />(Attached is a rather small patch tofix that! :) ) <br /><br />-- <br />Amit Langote <br /><div class="small"><br /><img src="/images/icon_attachment.gif"/> <strong>minor-xlog-comment.patch</strong> (914 bytes) <a href="http://postgresql.1045698.n5.nabble.com/attachment/5754017/0/minor-xlog-comment.patch"link="external" rel="nofollow"target="_top">Download Attachment</a></div><br /><hr align="left" width="300" /> View this message in context:<a href="http://postgresql.1045698.n5.nabble.com/Confusing-comment-in-xlog-c-or-am-I-missing-something-tp5754010p5754017.html">Re: Confusingcomment in xlog.c or am I missing something?</a><br /> Sent from the <a href="http://postgresql.1045698.n5.nabble.com/PostgreSQL-hackers-f1928748.html">PostgreSQL- hackers mailing list archive</a>at Nabble.com.<br />
Attached patch fixes this. -- Amit Langote
Attachment
On 02.05.2013 18:05, Amit Langote wrote: > Attached patch fixes this. ok, applied. - Heikki