[pgpool-hackers: 5] Re: Fix error on the lsn math

[Guillaume Lelarge]

On Tue, 2011-11-29 at 09:47 +0900, Tatsuo Ishii wrote:
> > On Mon, 2011-11-28 at 07:56 +0900, Tatsuo Ishii wrote:
> >> Guillaume,
> >> 
> >> About your commit in the subject.
> >> 
> >> Before it used 0xff000000 and changed by me according to the
> >> suggestion:
> >> 
> >> From: Anton Yuzhaninov
> >> To: pgpool-hackers at pgfoundry.org
> >> Date: Wed, 04 May 2011 19:56:54 +0400
> >> 
> >> > I have two questions/suggestions about replication lag check for
> >> > straming replication in pgpool-II.
> >> > 
> >> > 1. In text_to_lsn() there is formula
> >> > 
> >> > lsn = xlogid * 16 * 1024 * 1024 * 255 + xrecoff;
> >> > 
> >> > 16 * 1024 * 1024 * 255 = 4278190080 = 0xff000000
> >> > 
> >> > where this magic numbers come from?
> >> > 
> >> > I think multiplier should be
> >> > 0xffffffff - XLogSegSize.
> >> > 
> >> > In PostgreeSQL source this constant used in
> >> > src/include/access/xlog_internal.h
> >> > 
> >> > #define XLogSegsPerFile (((uint32) 0xffffffff) / XLogSegSize)
> >> > 
> >> > But in comment noted, that one segment at the end of each log file is
> >> > wasted
> >> > 
> >> > Each xlogid corresponds to (XLogSegsPerFile - 1) * XLogSegSize =
> >> > 0xffffffff - XLogSegSize bytes
> >> 
> >> I confirmed in the source code and decided to change "16 * 1024 * 1024
> >> * 255 = 4278190080 = 0xff000000" to 0xffffffff. Now you changed it
> >> to original constant. Do you have any point against his argument?
> > 
> > To be honest, I didn't know about Anton's point of view. I guess he's
> > wrong. See
> > http://eulerto.blogspot.com/2011/11/understanding-wal-nomenclature.html
> > for details. Many agree that we should use 0xff000000.
> 
> I think you are plain wrong.
> 
> 	lsn = xlogid * ((unsigned long long int)0xff000000 - WALSEGMENTSIZE) + xrecoff;
> 
> In the blog he said that correct formula should be:
> 
> FF000000 * xlog_master + offset_master
> 
> Here xlog_master = our xlogid, and offset_master = our xrecoff.
> So correct fomula for us is:
> 
> 	lsn = xlogid * ((unsigned long long int)0xff000000) + xrecoff;
> 
> or 
> 
>     lsn = xlogid * ((unsigned long long int)0xffffffff - WALSEGMENTSIZE) + xrecoff;
> 	(actually that was before you made change. Note that 0xffffffff - WALSEGMENTSIZE = 0xff0000).
> 
> In summary you subtracted too much.

You're right. My apologies to you and Anton, I was too quick on this
one. Patch reverted.


-- 
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com