Re: suppress automatic recovery after back crash

[moving from -performance to -hackers; original subject is: PostgreSQL
as a local in-memory cache]

On Thu, Jun 17, 2010 at 7:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>>> (a) and (d) are probably simple, if by "reprovisioning" you mean
>>> "rm -rf $PGDATA; initdb".
>
>> Exactly.  Followed by "scp database_image".  Or heck, just replacing the
>> whole VM.
>
> Right, that would work.  I don't think you really need to implement that
> inside Postgres.  I would envision having the startup script do it, ie
>
>        rm -rf $PGDATA
>        cp -pr prepared-database-image $PGDATA
>
>        # this loop exits when postmaster exits normally
>        while ! postmaster ...
>        do
>                rm -rf $PGDATA
>                cp -pr prepared-database-image $PGDATA
>        done
>
> Then all you need is a tweak to make the postmaster exit(1) after
> a crash instead of trying to launch recovery.

This seems useful to me so here's a patch to implement it.

There didn't seem to be a suitable GUC category for it, until I
noticed that we have a currently-undocumented GUC called
exit_on_error. I thought it might make sense to document both that
and this in a new section called "Error Handling".

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Jun 17, 2010 at 7:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Then all you need is a tweak to make the postmaster exit(1) after
>> a crash instead of trying to launch recovery.

> This seems useful to me so here's a patch to implement it.

Hm, is it useful in the absence of the other components of the proposed
feature?

One stylistic gripe:

@@ -80,6 +80,7 @@ enum config_group
COMPAT_OPTIONS,
COMPAT_OPTIONS_PREVIOUS,
COMPAT_OPTIONS_CLIENT,
+ ERROR_HANDLING,
PRESET_OPTIONS,
CUSTOM_OPTIONS,
DEVELOPER_OPTIONS

Please spell that "ERROR_HANDLING_OPTIONS", both for consistency with
the other enum members and to avoid likely conflicts with other uses of
such a generic-looking identifier.

regards, tom lane

On Sun, Jun 27, 2010 at 9:02 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Thu, Jun 17, 2010 at 7:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Then all you need is a tweak to make the postmaster exit(1) after
>>> a crash instead of trying to launch recovery.
>
>> This seems useful to me so here's a patch to implement it.
>
> Hm, is it useful in the absence of the other components of the proposed
> feature?

I think so. People are already using PostgreSQL as a feature-rich
cache; the point of the other changes Josh mentioned is just to make
it more performant. Specifically, he mentioned: (a) Eliminate WAL
logging entirely, (b) Eliminate checkpointing, and (c) Turn off the
background writer. I'm worked on unlogged tables, which will take us
about as far as we're likely to go in the direction of (a), per the
discussion on -performance. I haven't thought too much about (b) and
(c) so I'm not sure how involved that is, or how far we get just by
setting bgwriter_lru_maxpagess=0 as Greg Smith suggested, but, again,
it's just a performance optimization of something people are already
doing.

> One stylistic gripe:
>
> @@ -80,6 +80,7 @@ enum config_group
>        COMPAT_OPTIONS,
>        COMPAT_OPTIONS_PREVIOUS,
>        COMPAT_OPTIONS_CLIENT,
> +       ERROR_HANDLING,
>        PRESET_OPTIONS,
>        CUSTOM_OPTIONS,
>        DEVELOPER_OPTIONS
>
> Please spell that "ERROR_HANDLING_OPTIONS", both for consistency with
> the other enum members and to avoid likely conflicts with other uses of
> such a generic-looking identifier.

I mulled over which of those names was better; updated version,
reflecting your proposed naming, attached.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Mon, Jun 28, 2010 at 9:54 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> This seems useful to me so here's a patch to implement it.

+1

This would be very useful for people who want to give a clusterware
control of postgres.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Mon, Jun 28, 2010 at 9:09 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I mulled over which of those names was better; updated version,
> reflecting your proposed naming, attached.

I read the patch and found some small typos.

> + If true, any error will terminate the current session. Normally,
> + this is set to false, so that only FATAL errors will terminate the

"s/Normally/By default" seems better.

> + When set to true, which is the default, <productname>PostgreSQL</>
> + will automatically reinitialize after a backend crash. Leaving this
> + value set to true is normally the best way to maximize the availability
> + of the database. However, in some circumstances, such as when
> + <productname>PostgreSQL</> is being invoked by clusterware, it may be
> + useful to disable this behavior, so that the clusterware can gain
> + control and take any actions it deems appropriate.

We should add something like?:

---------
Even if this value is set to true, a backend crash during hot standby doesn't
reinitialize the database.
---------

> + /* ERROR_HANDING */
> + gettext_noop("Error Handling"),

You seems to have forgotten to reflect Tom's proposal here.

> #------------------------------------------------------------------------------
> +# ERROR HANDING
> +#------------------------------------------------------------------------------

Typo: s/HANDING/HANDLING

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Wed, Jul 14, 2010 at 3:41 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> I read the patch and found some small typos.

Thanks. Corrected version attached.

> We should add something like?:
>
> ---------
> Even if this value is set to true, a backend crash during hot standby doesn't
> reinitialize the database.
> ---------

Is that actually true? AFAICS, RecoveryError only gets set if the
*startup* process crashes.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Wed, Jul 14, 2010 at 11:59 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Jul 14, 2010 at 3:41 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> I read the patch and found some small typos.
>
> Thanks.  Corrected version attached.
>
>> We should add something like?:
>>
>> ---------
>> Even if this value is set to true, a backend crash during hot standby doesn't
>> reinitialize the database.
>> ---------
>
> Is that actually true? AFAICS, RecoveryError only gets set if the
> *startup* process crashes.

Oh, true. Sorry for noise.

I changed the status of the patch to "ready for committer".

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Sun, 2010-06-27 at 20:54 -0400, Robert Haas wrote:
> automatic_restart = true # reinitialize after backend crash?

"automatic_restart" makes me think "when does that happen?".

Can we call this "restart_after_crash"? Or similar. So we are explicit
about when the restart will kick in.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On Sun, 2010-06-27 at 20:54 -0400, Robert Haas wrote:
>> automatic_restart = true # reinitialize after backend crash?

> "automatic_restart" makes me think "when does that happen?".

> Can we call this "restart_after_crash"? Or similar.

+1. "automatic_restart" is close to content-free.

regards, tom lane

On Sat, Jul 17, 2010 at 10:16 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> On Sun, 2010-06-27 at 20:54 -0400, Robert Haas wrote:
>>> automatic_restart = true      # reinitialize after backend crash?
>
>> "automatic_restart" makes me think "when does that happen?".
>
>> Can we call this "restart_after_crash"? Or similar.
>
> +1.  "automatic_restart" is close to content-free.

OK, committed that way.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company