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. > > 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? > > I can see value in an option to exclude local buffers, since corruption > > there > > may be less exciting. ?It doesn't seem important for an initial patch, > > though. > > I'm continuing to exclude local buffers. Let me know if that should change. Seems reasonable. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
