Re: [REVIEW] Re: Compression of full-page-writes

On Mon, Feb 16, 2015 at 8:30 PM, Syed, Rahila <Rahila(dot)Syed(at)nttdata(dot)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