Re: Let's remove DSM_INPL_NONE.

At Wed, 28 Feb 2018 10:08:59 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180228010859(dot)GB1476(at)paquier(dot)xyz>
> On Tue, Feb 27, 2018 at 02:53:55PM -0500, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> >> On 2018-02-27 14:41:47 -0500, Tom Lane wrote:
> >>> What I didn't understand about it was what kind of testing this'd make
> >>> harder. If we desupport dynamic_shared_memory_type=none, there aren't
> >>> any code paths that need to cope with the case, and we should just
> >>> remove any code that thereby becomes unreachable.
> >
> >> What I'm concerned about isn't so much testing paths specific to
> >> dynamic_shared_memory_type=none, but paths where we currently need
> >> fallbacks for the case we couldn't actually allocate dynamic shared
> >> memory. Which I think we at least somewhat gracefully need to deal with.
> >
> > Ah. That's a fair point, but I do not think
> > dynamic_shared_memory_type=none is a good substitute for having a way to
> > provoke allocation failures. That doesn't let you test recovery from
> > situations where your first allocation works and second one fails, for
> > example; and cleanup from that sort of case is likely to be more
> > complicated than the trivial case.

On the other hand, dynamic_shared_memory_type=none is not a
proper means to silence DSM failures. If this patch causes some
problems, exactly the same things would have happened for the
setting dynamic_shared_memory_type *!=* none. If we need more
"graceful" behavior for some specific failure modes, it should be
amended separately.

> dynamic_shared_memory_type is used in six code paths where it is aimed
> at doing sanity checks:
...
> The point is that there are other control mechanisms for each one of
> them. Mainly, for the executor portion, the number of workers is
> controlled by other GUCs on planner-side.

If this means just that there should be any means other than the
variable to turn off user-visible parallel features:

> - Mute DSM initialization at postmaster startup.
> - Mute cleanup of previous DSM segments from a past postmaster.
These should be allowed unconditionally and should succeed.

> - Block backend startup if there is no DSM.
This is the result of any parallel activity, so this also should
be allowed unconditionally and should succeed.

> - Mute parallel query in planner.
max_parallel_workers_per_gather=0 kills it.
(min_parallel_*_scan_size or parallel_*_cost effectively can do
the similar.)

> - Mute parallel query for parallel btree builds.
max_parallel_maintenance_workers = 0 does.

> - Mute creation of parallel contexts in executor.
No gahter or gather merge results in this. So
max_parallel_workers_per_gather=0 also forces this.

The attached is the rebased version, on which the symbols are
renumbered 0-based as per comments.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center