Hi, Ivan! > 31 дек. 2017 г., в 22:30, Ivan Kartyshov <i.kartys...@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 by general 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 seems strange. 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 looking around 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.
