On Fri, Nov 29, 2019 at 11:39 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Thu, Oct 03, 2019 at 05:54:40PM +0900, Fujii Masao wrote: > > On Thu, Oct 3, 2019 at 1:57 PM Michael Paquier <mich...@paquier.xyz> wrote: > > > > > > On Thu, Oct 03, 2019 at 01:49:34PM +0900, Fujii Masao wrote: > > > > But this can cause subsequent recovery to always fail with > > > > invalid-pages error > > > > and the server not to start up. This is bad. So, to allviate the > > > > situation, > > > > I'm thinking it would be worth adding something like igore_invalid_pages > > > > developer parameter. When this parameter is set to true, the startup > > > > process > > > > always ignores invalid-pages errors. Thought? > > > > > > That could be helpful. > > > > So attached patch adds new developer GUC "ignore_invalid_pages". > > Setting ignore_invalid_pages to true causes the system > > to ignore the failure (but still report a warning), and continue recovery. > > > > I will add this to next CommitFest. > > No actual objections against this patch from me as a dev option.
Thanks for the review! Attached is the updated version of the patch. > + Detection of WAL records having references to invalid pages during > + recovery causes <productname>PostgreSQL</productname> to report > + an error, aborting the recovery. Setting > Well, that's not really an error. This triggers a PANIC, aka crashes > the server. And in this case the actual problem is that you may not > be able to move on with recovery when restarting the server again, > except if luck is on your side because you would continuously face > it.. So you're thinking that "report an error" should be changed to "trigger a PANIC"? Personally "report an error" sounds ok because PANIC is one of "error", I think. But if that misleads people, I will change the sentence. > + recovery. This behavior may <emphasis>cause crashes, data loss, > + propagate or hide corruption, or other serious problems</emphasis>. > Nit: indentation on the second line here. Yes, I fixed that. > + However, it may allow you to get past the error, finish the recovery, > + and cause the server to start up. > For consistency here I would suggest the second part of the sentence > to be "TO finish recovery, and TO cause the server to start up". Yes, I fixed that. > + The default setting is off, and it can only be set at server start. > Nit^2: Missing a <literal> markup for "off"? Yes, I fixed that. Regards, -- Fujii Masao
ignore_invalid_pages_v2.patch
Description: Binary data
