Re: Let's remove DSM_INPL_NONE. - Mailing list pgsql-hackers

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