On Fri, Jan 20, 2012 at 12:54 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > Thanks for the review! > > On Fri, Jan 20, 2012 at 8:15 PM, Simon Riggs <si...@2ndquadrant.com> wrote: >> I'm looking at this patch and wondering why we're doing so many >> press-ups to ensure full_page_writes parameter is on. This will still >> fail if you use a utility that removes the full page writes, but fail >> silently. >> >> I think it would be beneficial to explicitly check that all WAL >> records have full page writes actually attached to them until we >> achieve consistency. > > I agree that it's worth adding such a safeguard. That can be a self-contained > feature, so I'll submit a separate patch for that, to keep each patch small.
Maybe, but you mean do this now as well? Not sure I like silent errors. >> Surprised to see XLOG_FPW_CHANGE is there again after I objected to it >> and it was removed. Not sure why? We already track other parameters >> when they change, so I don't want to introduce a whole new WAL record >> for each new parameter whose change needs tracking. > > I revived that because whenever full_page_writes must be WAL-logged > or replayed, there is no need to WAL-log or replay the HS parameters. > The opposite is also true. Logging or replaying all of them every time > seems to be a bit useless, and to make the code unreadable. ISTM that > XLOG_FPW_CHANGE can make the code simpler and avoid adding useless > WAL activity by merging them into one WAL record. I don't agree, but for the sake of getting on with things I say this is minor so is no reason to block this. >> Please make a note for committer that wal version needs bumping. > > Okay, will add the note about bumping XLOG_PAGE_MAGIC. > >> I think its probably time to start a README.recovery to explain why >> this works the way it does. Other changes can then start to do that as >> well, so we can keep this to sane levels of complexity. > > In this CF, there are other patches which change recovery codes. So > I think that it's better to do that after all of them will have been > committed. > No need to hurry up to do that now. Agreed. Will proceed to final review and if all OK, commit. -- 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
