On Wed, Feb 8, 2012 at 3:24 AM, Noah Misch <n...@leadboat.com> wrote: > On Tue, Feb 07, 2012 at 08:58:59PM +0000, Simon Riggs wrote: >> On Thu, Jan 26, 2012 at 8:20 PM, Noah Misch <n...@leadboat.com> wrote: >> > On Wed, Jan 11, 2012 at 10:12:31PM +0000, Simon Riggs wrote: > >> > This patch uses FPIs to guard against torn hint writes, even when the >> > checksums are disabled. ?One could not simply condition them on the >> > page_checksums setting, though. ?Suppose page_checksums=off and we're >> > hinting >> > a page previously written with page_checksums=on. ?If that write tears, >> > leaving the checksum intact, that block will now fail verification. ?A >> > couple >> > of ideas come to mind. ?(a) Read the on-disk page and emit an FPI only if >> > the >> > old page had a checksum. ?(b) Add a checksumEnableLSN field to pg_control. >> > Whenever the cluster starts with checksums disabled, set the field to >> > InvalidXLogRecPtr. ?Whenever the cluster starts with checksums enabled and >> > the >> > field is InvalidXLogRecPtr, set the field to the next LSN. ?When a checksum >> > failure occurs in a page with LSN older than the stored one, emit either a >> > softer warning or no message at all. >> >> We can only change page_checksums at restart (now) so the above seems moot. >> >> If we crash with FPWs enabled we repair any torn pages. > > There's no live bug, but that comes at a high cost: the patch has us emit > full-page images for hint bit writes regardless of the page_checksums setting.
Sorry, I don't understand what you mean. I don't see any failure cases that require that. page_checksums can only change at a shutdown checkpoint, The statement "If that write tears, >> > leaving the checksum intact, that block will now fail verification." cannot happen, ISTM. If we write out a block we update the checksum if page_checksums is set, or we clear it. If we experience a torn page at crash, the FPI corrects that, so the checksum never does fail verification. We only need to write a FPI when we write with checksums. If that's wrong, please explain a failure case in detail. >> > PageSetLSN() is not atomic, so the shared buffer content lock we'll be >> > holding >> > is insufficient. >> >> Am serialising this by only writing PageLSN while holding buf hdr lock. > > That means also taking the buffer header spinlock in every PageGetLSN() caller > holding only a shared buffer content lock. Do you think that will pay off, > versus the settled pattern of trading here your shared buffer content lock for > an exclusive one? Yes, I think it will pay off. This is the only code location where we do that, and we are already taking the buffer header lock, so there is effectively no overhead. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers