Re: [Patch] Checksums for SLRU files

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Ivan Kartyshov <i(dot)kartyshov(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [Patch] Checksums for SLRU files
Date: 2018-01-01 18:19:55
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi, Ivan!
> 31 дек. 2017 г., в 22:30, Ivan Kartyshov <i(dot)kartyshov(at)postgrespro(dot)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.

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Gavin Flower 2018-01-01 19:54:25 Re: What does Time.MAX_VALUE actually represent?
Previous Message Noah Misch 2018-01-01 18:18:01 Re: Increasing timeout of poll_query_until for TAP tests