Let's remove DSM_INPL_NONE.

Поиск
Список
Период
Сортировка
От Kyotaro HORIGUCHI
Тема Let's remove DSM_INPL_NONE.
Дата
Msg-id 20180215.195857.109347328.horiguchi.kyotaro@lab.ntt.co.jp
обсуждение исходный текст
Ответы Re: Let's remove DSM_INPL_NONE.
Re: Let's remove DSM_INPL_NONE.
Re: Let's remove DSM_INPL_NONE.
Список pgsql-hackers
Hello.

As in the other threads[1][2], we have had several good reasons
to remove DSM_IMPL_NONE in PG11. The attached patch doesn that.

[1] https://www.postgresql.org/message-id/CA+Tgmoa0e23YC3SCwB85Yf5oUTRyirV=Sq_rXYxaXABLdPpjoA@mail.gmail.com

[2] https://www.postgresql.org/message-id/20180214.103858.02713585.horiguchi.kyotaro@lab.ntt.co.jp


It is amost a just-to-delete work but I see two issues raised so
far.

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

[4] Commit: d41ab71712a4457ed39d5471b23949872ac91def

[5] https://www.postgresql.org/message-id/20180209.170823.42008365.horiguchi.kyotaro@lab.ntt.co.jp


2. Server may chooose unusable DSM implementation while initdb probing. 

https://www.postgresql.org/message-id/20180214033248.GA1993@paquier.xyz

> 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].

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 44411ee17e881d1ee42fb91106d62aa97a7e45c9 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. Assuming none hinder us from using it
for core features.  Just removing the option leads to intermittent
failure of regression test on some platforms due to insufficient
max_connection on initdb. This patch requires the patch that increases
the minimum fallback value of max_connection of initdb.
---
 doc/src/sgml/config.sgml                      |  6 +++---
 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 -
 8 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c45979d..c2d86b3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1601,9 +1601,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/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 9d4efc0..4d27241 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -162,13 +162,6 @@ CreateParallelContext(const char *library_name, const char *function_name,
     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
      * parallel-aware.  Utility statement callers may ask us to ignore this
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 3e8cd14..368fec4 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -303,7 +303,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 &&
@@ -5825,8 +5824,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 f1f75b7..9629f22 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 67e76b9..8239903 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -105,7 +105,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}
 };
 
@@ -209,8 +208,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 9a35355..0d60193 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -131,7 +131,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 2efd3b7..d520a39 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -933,6 +933,16 @@ test_config_settings(void)
                 ok_buffers = 0;
 
 
+    /*
+     * Server doesn't confirm that the server-default DSM implementation is
+     * actually workable. Choose a fine one for probing then it is used on the
+     * new database.
+     */
+    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);
 
@@ -945,10 +955,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)
@@ -980,10 +991,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)
@@ -995,11 +1007,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 0e5730f..e7acdff 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.9.2


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Ashutosh Bapat
Дата:
Сообщение: Re: [HACKERS] why not parallel seq scan for slow functions
Следующее
От: Kyotaro HORIGUCHI
Дата:
Сообщение: Removing useless #include's.