Re: [Patch] Checksums for SLRU files

Hi, Ivan!
> 31 дек. 2017 г., в 22:30, Ivan Kartyshov <i.kartyshov@postgrespro.ru> написал(а):
>
> Hello, I`d like to show my implementation of SLRU file protection with checksums.
> .....
> I would like to hear your thoughts over my patch.

As far as I can see, the patch solves problem of hardware corruption in SLRU.
This seems a valid concern. I've tried to understand your patch and few questions arose which I could not answer
myself.

1. Why do you propose different GUC besides ignore_checksum_failure? What is scenario of it's use which is not covered
bygeneral GUC switch? 
2. What is performance penalty of having this checksums?

Besides this, some things seems suspicious to me:
1. This comment seems excessive. I'd leave just first one first line.
+/*
+ * GUC variable
+ * Set from backend:
+ * alter system set ignore_slru_checksum_failure = on/off;
+ * select pg_reload_conf();
+ */
2. Functions pg_checksum_slru_page() and pg_getchecksum_slru_page() operate with two bytes instead of uint16. This
seemsstrange. 
3. This line
checksum = (pg_checksum_block(page, BLCKSZ) % 65535) + 1;
Need to share comment with previous function (pg_checksum_page()). +1 was a tough thing for me to understand before
lookingaround and reading those comments. 
4. I could not understand purpose of this expression
page[BLCKSZ - 1] & 0X00FF

Happy New Year :) Keep up good work.

Best regards, Andrey Borodin.