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

Thank you for the notice.

At Mon, 9 Jul 2018 12:30:22 +0300, Arthur Zakirov <a.zakirov@postgrespro.ru> wrote in
<20180709093021.GA9309@zakirov.localdomain>
> Hello,
> 
> On Mon, Jul 09, 2018 at 06:07:24PM +0900, Kyotaro HORIGUCHI wrote:
> > The new version v4 is changed in the following points.
> > 
> > - Don't renumber the DSM_IMPL symbols, just removed _NONE.
> > 
> > - Rewrote the pointed comment.
> > 
> > - Revised the commit message removing a mention to an
> >   already-committed patch.
> > 
> > - (and rebased)
> 
> Just a little note. In parallel.sgml it is still mentioned that
> dynamic_shared_memory_type accepts 'none' value:
> 
> > <xref linkend="guc-dynamic-shared-memory-type"/> must be set to a
> > value other than <literal>none</literal>.

Oops! Thanks. Just removed it altogether and I didn't find
another instance.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 1ff3e3ca9cd971cc75922ecd1507304ee2c27fd4 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 15 Feb 2018 11:22:50 +0900
Subject: [PATCH] Remove dynamic_shared_memroy_type=none

Nowadays PostgreSQL on all supported platform offers any kind of
dynamic shared memory feature. Having the choice of none hinder us
from using it in core features. It is used only in initdb probing for
max connections and we are assuming that any kind of DSM is available
in genrated dtabase cluster. This patch removes the choice at
all. Since just removing it can cause initdb failure in certain cases,
initdb now identifies an available DSM implementation prior to the
probing with this patch.
---
 doc/src/sgml/config.sgml                      |  6 +++---
 doc/src/sgml/parallel.sgml                    |  8 --------
 src/backend/access/transam/parallel.c         |  7 -------
 src/backend/optimizer/plan/planner.c          |  4 +---
 src/backend/storage/ipc/dsm.c                 | 15 ---------------
 src/backend/storage/ipc/dsm_impl.c            |  3 ---
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 src/bin/initdb/initdb.c                       | 21 ++++++++++++++-------
 src/include/storage/dsm_impl.h                |  1 -
 9 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 5b913f00c1..e307bb4e8e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1662,9 +1662,9 @@ include_dir 'conf.d'
         should use.  Possible values are <literal>posix</literal> (for POSIX shared
         memory allocated using <literal>shm_open</literal>), <literal>sysv</literal>
         (for System V shared memory allocated via <literal>shmget</literal>),
-        <literal>windows</literal> (for Windows shared memory), <literal>mmap</literal>
-        (to simulate shared memory using memory-mapped files stored in the
-        data directory), and <literal>none</literal> (to disable this feature).
+        <literal>windows</literal> (for Windows shared memory),
+        and <literal>mmap</literal> (to simulate shared memory using
+        memory-mapped files stored in the data directory).
         Not all values are supported on all platforms; the first supported
         option is the default for that platform.  The use of the
         <literal>mmap</literal> option, which is not the default on any platform,
diff --git a/doc/src/sgml/parallel.sgml b/doc/src/sgml/parallel.sgml
index dd7834a763..e9a015ecd3 100644
--- a/doc/src/sgml/parallel.sgml
+++ b/doc/src/sgml/parallel.sgml
@@ -124,14 +124,6 @@ EXPLAIN SELECT * FROM pgbench_accounts WHERE filler LIKE '%x%';
         configured via <varname>max_parallel_workers_per_gather</varname>.
       </para>
     </listitem>
-
-    <listitem>
-      <para>
-        <xref linkend="guc-dynamic-shared-memory-type"/> must be set to a
-        value other than <literal>none</literal>.  Parallel query requires dynamic
-        shared memory in order to pass data between cooperating processes.
-      </para>
-    </listitem>
   </itemizedlist>
 
   <para>
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 1d631b7275..4e32cfff50 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -161,13 +161,6 @@ CreateParallelContext(const char *library_name, const char *function_name,
     /* Number of workers should be non-negative. */
     Assert(nworkers >= 0);
 
-    /*
-     * If dynamic shared memory is not available, we won't be able to use
-     * background workers.
-     */
-    if (dynamic_shared_memory_type == DSM_IMPL_NONE)
-        nworkers = 0;
-
     /*
      * If we are running under serializable isolation, we can't use parallel
      * workers, at least not until somebody enhances that mechanism to be
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index fd45c9767d..eeebf775a4 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -335,7 +335,6 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
      */
     if ((cursorOptions & CURSOR_OPT_PARALLEL_OK) != 0 &&
         IsUnderPostmaster &&
-        dynamic_shared_memory_type != DSM_IMPL_NONE &&
         parse->commandType == CMD_SELECT &&
         !parse->hasModifyingCTE &&
         max_parallel_workers_per_gather > 0 &&
@@ -6050,8 +6049,7 @@ plan_create_index_workers(Oid tableOid, Oid indexOid)
     double        allvisfrac;
 
     /* Return immediately when parallelism disabled */
-    if (dynamic_shared_memory_type == DSM_IMPL_NONE ||
-        max_parallel_maintenance_workers == 0)
+    if (max_parallel_maintenance_workers == 0)
         return 0;
 
     /* Set up largely-dummy planner state */
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index f1f75b73f5..9629f22f7a 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -150,10 +150,6 @@ dsm_postmaster_startup(PGShmemHeader *shim)
 
     Assert(!IsUnderPostmaster);
 
-    /* If dynamic shared memory is disabled, there's nothing to do. */
-    if (dynamic_shared_memory_type == DSM_IMPL_NONE)
-        return;
-
     /*
      * If we're using the mmap implementations, clean up any leftovers.
      * Cleanup isn't needed on Windows, and happens earlier in startup for
@@ -219,10 +215,6 @@ dsm_cleanup_using_control_segment(dsm_handle old_control_handle)
     uint32        i;
     dsm_control_header *old_control;
 
-    /* If dynamic shared memory is disabled, there's nothing to do. */
-    if (dynamic_shared_memory_type == DSM_IMPL_NONE)
-        return;
-
     /*
      * Try to attach the segment.  If this fails, it probably just means that
      * the operating system has been rebooted and the segment no longer
@@ -391,13 +383,6 @@ dsm_postmaster_shutdown(int code, Datum arg)
 static void
 dsm_backend_startup(void)
 {
-    /* If dynamic shared memory is disabled, reject this. */
-    if (dynamic_shared_memory_type == DSM_IMPL_NONE)
-        ereport(ERROR,
-                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                 errmsg("dynamic shared memory is disabled"),
-                 errhint("Set dynamic_shared_memory_type to a value other than \"none\".")));
-
 #ifdef EXEC_BACKEND
     {
         void       *control_address = NULL;
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index c6382ec031..77e1bab54b 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -106,7 +106,6 @@ const struct config_enum_entry dynamic_shared_memory_options[] = {
 #ifdef USE_DSM_MMAP
     {"mmap", DSM_IMPL_MMAP, false},
 #endif
-    {"none", DSM_IMPL_NONE, false},
     {NULL, 0, false}
 };
 
@@ -210,8 +209,6 @@ dsm_impl_can_resize(void)
 {
     switch (dynamic_shared_memory_type)
     {
-        case DSM_IMPL_NONE:
-            return false;
         case DSM_IMPL_POSIX:
             return true;
         case DSM_IMPL_SYSV:
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 9e39baf466..657c3f81f8 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -133,7 +133,6 @@
                     #   sysv
                     #   windows
                     #   mmap
-                    # use none to disable dynamic shared memory
                     # (change requires restart)
 
 # - Disk -
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ae22e7d9fb..75faac7d8c 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -984,6 +984,16 @@ test_config_settings(void)
                 ok_buffers = 0;
 
 
+    /*
+     * The bootstrapping postgresql may use unworkable DSM implementation by
+     * default. Identify a workable one now and specify it in the probing
+     * steps below.
+     */
+    printf(_("selecting dynamic shared memory implementation ... "));
+    fflush(stdout);
+    dynamic_shared_memory_type = choose_dsm_implementation();
+    printf("%s\n", dynamic_shared_memory_type);
+
     printf(_("selecting default max_connections ... "));
     fflush(stdout);
 
@@ -996,10 +1006,11 @@ test_config_settings(void)
                  "\"%s\" --boot -x0 %s "
                  "-c max_connections=%d "
                  "-c shared_buffers=%d "
-                 "-c dynamic_shared_memory_type=none "
+                 "-c dynamic_shared_memory_type=%s "
                  "< \"%s\" > \"%s\" 2>&1",
                  backend_exec, boot_options,
                  test_conns, test_buffs,
+                 dynamic_shared_memory_type,
                  DEVNULL, DEVNULL);
         status = system(cmd);
         if (status == 0)
@@ -1031,10 +1042,11 @@ test_config_settings(void)
                  "\"%s\" --boot -x0 %s "
                  "-c max_connections=%d "
                  "-c shared_buffers=%d "
-                 "-c dynamic_shared_memory_type=none "
+                 "-c dynamic_shared_memory_type=%s "
                  "< \"%s\" > \"%s\" 2>&1",
                  backend_exec, boot_options,
                  n_connections, test_buffs,
+                 dynamic_shared_memory_type,
                  DEVNULL, DEVNULL);
         status = system(cmd);
         if (status == 0)
@@ -1046,11 +1058,6 @@ test_config_settings(void)
         printf("%dMB\n", (n_buffers * (BLCKSZ / 1024)) / 1024);
     else
         printf("%dkB\n", n_buffers * (BLCKSZ / 1024));
-
-    printf(_("selecting dynamic shared memory implementation ... "));
-    fflush(stdout);
-    dynamic_shared_memory_type = choose_dsm_implementation();
-    printf("%s\n", dynamic_shared_memory_type);
 }
 
 /*
diff --git a/src/include/storage/dsm_impl.h b/src/include/storage/dsm_impl.h
index 0e5730f7c5..e7acdff355 100644
--- a/src/include/storage/dsm_impl.h
+++ 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
-- 
2.16.3