On 2013-04-05 16:29:47 -0700, Jeff Davis wrote: > On Fri, 2013-04-05 at 15:09 +0200, Andres Freund wrote: > > How does the attached version look? I verified that it survives > > recovery, but not more. > > Comments: > > * Regarding full page writes, we can: > - always write full pages (as in your current patch), regardless of > the current settings > - take WALInsertLock briefly to get the current settings from XLogCtl > - always calculate the full page record, but in XLogInsert, if it > happens to be a HINT record, and full page writes are not necessary, > then discard it (right now I somewhat favor this option but I haven't > looked at the details)
I feel pretty strongly that we shouldn't add any such complications to XLogInsert() itself, its complicated enough already and it should be made simpler, not more complicated. I think we can just make up the rule that changing full page writes also requires SpinLockAcquire(&xlogctl->info_lck);. Then its easy enough. And it can hardly be a performance bottleneck given how infrequently its modified. In retrospect I think making up the rule that full_page_writes changes imply a checkpoint would have made things easier performance and codewise. > * typo in "Backup blocks are not used in **xlog xlog** records" Thats just me being "funny" after some long days ;). See its an 'xlog' 'xlog record'. > * To get the appropriate setting for buffer_std, we should pass it down > through MarkBufferDirty as an extra flag, I think. Or just declare as part of the api that only std buffers are allowed to be passed down. After a quick look it looks like all callers use enough of the standard page layout to make that viable. But that really was just a quick look. > * I'm a bit worried that we'll need a cleanup lock when restoring an FSM > page. What do you think? I don't yet see why, while recovery is ongoing there shouldn't be anyone doing anything concurrently to it but startup itself? > * In xlog_redo, it seemed slightly awkward to call XLogRecGetData twice. > Merely a matter of preference but I thought I would mention it. Youre absolutely right, memcpy should have gotten passed 'data', not XLogRecGetData(). > > Jeff, any chance you can run this for a round with your suite? > > Yes. I don't have a rigorous test suite, but I'll do some manual tests > and walk through it with gdb. Heh, in this and only this I was talking to Jeff Janes. Strangely I never had noticed that you share the same name ;) Thanks! Andres Freund -- Andres Freund 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
