Re: Miscalculation in IsCheckpointOnSchedule() - Mailing list pgsql-patches

From Heikki Linnakangas
Subject Re: Miscalculation in IsCheckpointOnSchedule()
Date
Msg-id 473AE03C.9040406@enterprisedb.com
Whole thread Raw
In response to Re: Miscalculation in IsCheckpointOnSchedule()  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Miscalculation in IsCheckpointOnSchedule()  (Heikki Linnakangas <heikki@enterprisedb.com>)
List pgsql-patches
Tom Lane wrote:
> ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
>> -         ((double) (int32) (recptr.xrecoff - ckpt_start_recptr.xrecoff)) / XLogSegSize) /
>> +         ((double) recptr.xrecoff - (double) ckpt_start_recptr.xrecoff) / XLogSegSize) /
>
> Surely this makes matters worse, not better.  What happens near a segment
> boundary crossing?

Hmm. There seems to be another little bug in there. XLogSegsPerFile is
defined as 0xffffffff/XLogSegSize, which is 255 with default settings.
It should be 256. That leads to negative elapsed_xlogs estimates at xlog
file boundaries. XLogCheckpointNeeded suffers from it too; the number of
segments consumed since last checkpoint is off by one per each xlogid,
so we trigger the checkpoint a little bit too late.

Otherwise, the patch looks good to me. I also tested the calculation
with a little C program (attached), and it seems to work on segment and
xlog file boundaries just fine.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
#include <stdio.h>
#include <stdlib.h>

typedef unsigned char uint8;    /* == 8 bits */
typedef unsigned short uint16;    /* == 16 bits */
typedef unsigned int uint32;    /* == 32 bits */

typedef signed char int8;        /* == 8 bits */
typedef signed short int16;        /* == 16 bits */
typedef signed int int32;        /* == 32 bits */

#define XLOG_SEG_SIZE    (16*1024*1024)
#define XLogSegSize        ((uint32) XLOG_SEG_SIZE)
#define XLogSegsPerFile (((uint32) 0xffffffff) / XLogSegSize)

int main(int argc, char **argv)
{
  double elapsed_xlogs;
  uint32 cur_xrecoff, cur_xlogid;
  uint32 ckpt_xrecoff, ckpt_xlogid;
  double a, b;

  cur_xlogid = strtoul(argv[1], NULL, 10);
  cur_xrecoff = strtoul(argv[2], NULL, 10);

  ckpt_xlogid = strtoul(argv[3], NULL, 10);
  ckpt_xrecoff = strtoul(argv[4], NULL, 10);

  //a = (cur_xlogid - ckpt_xlogid) * XLogSegsPerFile;

  //b = (((double) cur_xrecoff) - ((double) ckpt_xrecoff)) / ((double )XLogSegSize);

  //a = ((double) (int32) (cur_xlogid - ckpt_xlogid)) * XLogSegsPerFile;
  //b  = ((double) (int32) (cur_xrecoff - ckpt_xrecoff)) / XLogSegSize;
  a = (cur_xlogid - ckpt_xlogid) * XLogSegsPerFile;
  b = ((double) cur_xrecoff - (double) ckpt_xrecoff) / XLogSegSize;

  elapsed_xlogs = a + b;


  printf("XLogSegsPerFile: %d\n", XLogSegsPerFile);
  printf("xrecoff: %u %f\n", ckpt_xrecoff, (double) ckpt_xrecoff);
  printf("a: %f\n", a);
  printf("b: %f\n", b);
  printf("elapsed_xlogs = %f\n", elapsed_xlogs);
}


pgsql-patches by date:

Previous
From: Jan Urbański
Date:
Subject: Re: a tsearch2 (8.2.4) dictionary that only filters out stopwords
Next
From: Heikki Linnakangas
Date:
Subject: Re: Miscalculation in IsCheckpointOnSchedule()