Обсуждение: [15] Custom WAL resource managers, single user mode, and recovery

Single user mode doesn't process shared_preload_libraries, which is a
problem if custom WAL resource managers are in use and recovery is
required.

I attached a simple patch to process shared_preload_libraries even in
single user mode.

If a user has custom table access methods, those are likely to be
required during single user mode anyway (to VACUUM, for instance), so
we need to be able to load modules during single user mode. I can't
think of a reason specifically why we can't process
shared_preload_libraries. Thoughts?

Regards,
    Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:
> If a user has custom table access methods, those are likely to be
> required during single user mode anyway (to VACUUM, for instance), so
> we need to be able to load modules during single user mode. I can't
> think of a reason specifically why we can't process
> shared_preload_libraries. Thoughts?

Hmm.  There may have been some idea that a library might fail in
single-user mode, and/or have been the cause of you needing to
use single-user in the first place.  But if so, you can always
edit it out of shared_preload_libraries.  I agree that not
having the option to load it isn't great.

I think that the patch might be missing some stuff.  In postmaster.c,
there are several steps after process_shared_preload_libraries()
that look to be there to support loadable libraries, such as
process_shmem_requests().  Wouldn't it be reasonable to expect
that a loadable library would malfunction without those?

(I wonder if we shouldn't refactor this so that the postmaster
and standalone mode share more of the initialization logic.
Keeping these bits in sync seems unlikely to happen otherwise.)

            regards, tom lane

Hi,

On 2022-07-14 20:48:57 -0400, Tom Lane wrote:
> Jeff Davis <pgsql@j-davis.com> writes:
> > If a user has custom table access methods, those are likely to be
> > required during single user mode anyway (to VACUUM, for instance), so
> > we need to be able to load modules during single user mode. I can't
> > think of a reason specifically why we can't process
> > shared_preload_libraries. Thoughts?
> 
> Hmm.  There may have been some idea that a library might fail in
> single-user mode, and/or have been the cause of you needing to
> use single-user in the first place.  But if so, you can always
> edit it out of shared_preload_libraries.  I agree that not
> having the option to load it isn't great.

+1


> I think that the patch might be missing some stuff.  In postmaster.c,
> there are several steps after process_shared_preload_libraries()
> that look to be there to support loadable libraries, such as
> process_shmem_requests().  Wouldn't it be reasonable to expect
> that a loadable library would malfunction without those?

+1


> (I wonder if we shouldn't refactor this so that the postmaster
> and standalone mode share more of the initialization logic.
> Keeping these bits in sync seems unlikely to happen otherwise.)

Yes, that might be worthwhile. OTOH, I wonder if we should spend that time to
remove single user mode instead - the architectural complexity really doesn't
seem worth it anymore, and IMO my prototype from a few months back showed that
it's feasible.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2022-07-14 20:48:57 -0400, Tom Lane wrote:
>> (I wonder if we shouldn't refactor this so that the postmaster
>> and standalone mode share more of the initialization logic.
>> Keeping these bits in sync seems unlikely to happen otherwise.)

> Yes, that might be worthwhile. OTOH, I wonder if we should spend that time to
> remove single user mode instead - the architectural complexity really doesn't
> seem worth it anymore, and IMO my prototype from a few months back showed that
> it's feasible.

I dunno ... if your DB is in bad enough shape that you need to resort
to single-user mode, you probably don't want any more moving parts
in the system than you absolutely have to have.  Autovacuum, custom
background workers, and the like are going to be counterproductive.

            regards, tom lane

Hi,

On 2022-07-14 23:42:56 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2022-07-14 20:48:57 -0400, Tom Lane wrote:
> >> (I wonder if we shouldn't refactor this so that the postmaster
> >> and standalone mode share more of the initialization logic.
> >> Keeping these bits in sync seems unlikely to happen otherwise.)
> 
> > Yes, that might be worthwhile. OTOH, I wonder if we should spend that time to
> > remove single user mode instead - the architectural complexity really doesn't
> > seem worth it anymore, and IMO my prototype from a few months back showed that
> > it's feasible.
> 
> I dunno ... if your DB is in bad enough shape that you need to resort
> to single-user mode, you probably don't want any more moving parts
> in the system than you absolutely have to have.

Well, right now the main reason people need single user is anti-wraparound
stuff. And there it's actively harmful (requiring a shutdown checkpoint,
emptying shared buffers, foreground checkpoints, foreground writing of all
WAL, etc).

It's also not comforting to hit a lot of codepaths that are exercised rarely,
when things already have gone pear-shaped. We've had plenty bugs - in
important paths like releasing lwlocks in case of errors - that were single
user specific.


> Autovacuum, custom background workers, and the like are going to be
> counterproductive.

I think it'd be better to address those with a GUC more squarely aimed at
disabling systems you don't need when on a corrupted cluster.

Greetings,

Andres Freund

Hi,

On 2022-07-15 09:00:40 +0200, Alexander Kukushkin wrote:
> On Fri, 15 Jul 2022 at 05:58, Andres Freund <andres@anarazel.de> wrote:
> 
> Well, right now the main reason people need single user is anti-wraparound
> > stuff. And there it's actively harmful (requiring a shutdown checkpoint,
> > emptying shared buffers, foreground checkpoints, foreground writing of all
> > WAL, etc).
> >
> 
> No, the single-user mode is also required to ensure a clean shutdown before
> pg_rewind.

I don't see why that requires single user mode? It's used, sure, but that
looks almost trival to change.

Greetings,

Andres Freund

On Thu, 2022-07-14 at 20:48 -0400, Tom Lane wrote:
> I think that the patch might be missing some stuff.  In postmaster.c,
> there are several steps after process_shared_preload_libraries()
> that look to be there to support loadable libraries, such as
> process_shmem_requests().  Wouldn't it be reasonable to expect
> that a loadable library would malfunction without those?

New patch attached.

This can also be a problem for custom table AMs, if the AM is expecting
to be loaded in shared_preload_libraries. For instance, it would be
reasonable for a module to have in _PG_init() something like:

  if (!process_shared_preload_libraries_in_progress)
    elog(ERROR, "module must be loaded in shared_preload_libraries");

to ensure hooks are properly configured at the right time. But if the
module does something like that, and also implements a custom table AM,
then it breaks single-user mode entirely because you can no longer do a
database-wide VACUUM.

I'm debating whether to backport this just to v15, where custom wal
resource managers were introduced, or all the way back to v12, where
custom table AMs were introduced. I'm inclined to just go to 15,
because the issue with table AMs is more of a gotcha than a real bug.

Regards,
    Jeff Davis

On Fri, Jul 15, 2022 at 12:58 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2022-07-14 23:42:56 -0400, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > On 2022-07-14 20:48:57 -0400, Tom Lane wrote:
> > >> (I wonder if we shouldn't refactor this so that the postmaster
> > >> and standalone mode share more of the initialization logic.
> > >> Keeping these bits in sync seems unlikely to happen otherwise.)
> >
> > > Yes, that might be worthwhile. OTOH, I wonder if we should spend that time to
> > > remove single user mode instead - the architectural complexity really doesn't
> > > seem worth it anymore, and IMO my prototype from a few months back showed that
> > > it's feasible.
> >
> > I dunno ... if your DB is in bad enough shape that you need to resort
> > to single-user mode, you probably don't want any more moving parts
> > in the system than you absolutely have to have.
>
> Well, right now the main reason people need single user is anti-wraparound
> stuff. And there it's actively harmful (requiring a shutdown checkpoint,
> emptying shared buffers, foreground checkpoints, foreground writing of all
> WAL, etc).

IIUC single user mode is not necessarily required for anti-wraparound
vacuum cases since in failsafe mode (or disabling heap truncation etc)
we can vacuum table and advance relfrozenxid without XID allocation.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/