On Mon, Feb 16, 2015 at 8:30 PM, Syed, Rahila <rahila.s...@nttdata.com> wrote:
> > Regarding the sanity checks that have been added recently. I think that > they are useful but I am suspecting as well that only a check on the record > CRC is done because that's reliable enough and not doing those checks > accelerates a bit replay. So I am thinking that we should simply replace > >them by assertions. > > Removing the checks makes sense as CRC ensures correctness . Moreover ,as > error message for invalid length of record is present in the code , > messages for invalid block length can be redundant. > > Checks have been replaced by assertions in the attached patch. > After more thinking, we may as well simply remove them, an error with CRC having high chances to complain before reaching this point... > Current > > if ((hole_length != 0) && > > + (*len >= orig_len - > SizeOfXLogRecordBlockImageCompressionInfo)) > > + return false; > > +return true > This makes sense. Nitpicking 1: + Assert(!(blk->with_hole == 1 && blk->hole_offset <= 0)); Double-space here. Nitpicking 2: char * page This should be rewritten as char *page, the "*" being assigned with the variable name. -- Michael
