Re: Let's remove DSM_INPL_NONE.

On Thu, Feb 15, 2018 at 07:58:57PM +0900, Kyotaro HORIGUCHI wrote:
> It is amost a just-to-delete work but I see two issues raised so
> far.

The patch looks good as-is. This simplifies a couple of code paths
deciding if parallel queries can be used or not, so future features in
need of doing the same decision-making won't fall in the same trap as
the recent parellel btree builds. So I am +1 for having the buildfarm
express its opinion about it.

> 1. That change can raise regression test failure on some
> buildfarm machines[3].
>
> The reason is that initdb creates a database with
> max_connection=10 from DSM failure caused by semaphore exhaustion
> , even though regression test requires at least 20
> connections. At that time it was "fixed" by setting
> dynamic_shared_memory_type=none while detecting the number of
> usable max_connections and shared buffers[4]. Regardless of
> whether the probing was succeeded or not, initdb finally creats a
> database on that any DSM implementation is activated, since
> initdb doesn't choose "none". Finally the test items for parallel
> query actually suceeded.
>
> For the reason above, I believe just increasing the minimum
> fallback value of max_connections to 20 will work[5]. Anyway it
> is a problem to be fixed that initdb creates an inexecutable
> database if it uses the fallback values of max_connections=10.
>
> [3]
> https://www.postgresql.org/message-id/CA+TgmoYHiiGrcvSSJhmbSEBMoF2zX_9_9rWd75Cwvu99YrDxew@mail.gmail.com

Do actually buildfarm machines select max_connections = 10 now? We
would have surely seen failures since max_wal_senders has its default
value set to 10 as well. Hence this is a separate problem.

> [4] Commit: d41ab71712a4457ed39d5471b23949872ac91def
>
> [5] https://www.postgresql.org/message-id/20180209.170823.42008365.horiguchi.kyotaro@lab.ntt.co.jp
>
>
> > Feel free to. Be just careful that the connection attempts in
> > test_config_settings() should try to use the DSM implementation defined
> > by choose_dsm_implementation().
>
> Thank you for the advice. The probing fails with the default
> setting if posix shared memory somehow fails on a platform like
> Linux that is usually expected to have it. It's enough to choose
> the implementation before probing. Some messages from initdb are
> transposed as the result.
>
> | creating directory /home/horiguti/data/data_ndsm ... ok
> | creating subdirectories ... ok
> | + selecting dynamic shared memory implementation ... posix
> | selecting default max_connections ... 100
> | selecting default shared_buffers ... 128MB
> | - selecting dynamic shared memory implementation ... posix
>
> Even though probing can end with failure in the case of [3], it
> will not be a problem with the patch [4].

That's fine by me, as this is actually the purpose of your patch.

+++ b/src/include/storage/dsm_impl.h
@@ -14,7 +14,6 @@
#define DSM_IMPL_H

/* Dynamic shared memory implementations. */
-#define DSM_IMPL_NONE 0
#define DSM_IMPL_POSIX 1
#define DSM_IMPL_SYSV 2
#define DSM_IMPL_WINDOWS 3
You may as well assign numbers from 0 here and reorder the whole set.
--
Michael