Thread: Let's remove DSM_INPL_NONE.
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
Hi, On 2018-02-15 19:58:57 +0900, Kyotaro HORIGUCHI wrote: > 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 Hm, I'm not quite convinced by this. Seems to make testing a bunch of codepaths harder. I think it's fine to say that pg doesn't work correctly with them disabled though. - Andres
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
Attachment
On Thu, Feb 15, 2018 at 10:00:39AM -0800, Andres Freund wrote: > Hm, I'm not quite convinced by this. Seems to make testing a bunch of > codepaths harder. I think it's fine to say that pg doesn't work > correctly with them disabled though. Well, for what it's worth that's one thing less to be forgotten when implementing features related to parallel query. That's the lesson 88ef48c is telling us here, much unlikely anybody would disable it though after an initdb. -- Michael
Attachment
Hello, thank you for the comment. At Fri, 16 Feb 2018 13:07:08 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180216040708.GA1174@paquier.xyz> > 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. Agreed. I'd like to see how buildfarm machines respond to this. > > 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. Even not being pretty confident, that's because the current initdb runs probing with dynamic_s_m_type=none. So the same BF animal can fail if initdb probes with dynamic_s_m_type=sysv and semaphore is exchausted at the time. > > [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. The reason of that is I prefered to leave 0 as unused default value. But it doesn't have significance after a clean build. I'm fine with reordering (or renumbering) them. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
On Thu, Feb 15, 2018 at 5:58 AM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > It is amost a just-to-delete work but I see two issues raised so > far. I think the two issues you are pointing out are the same issue -- namely, if initdb probes for a max_connections setting with an invalid DSM implementation configured, it will fail the test for every value of max_connections and thus select the lowest possible value. The solution to this is presumably just to make sure we choose the DSM implementation before we do the max_connections probing so that we can pass through the correct value there, which I think is what your patch does. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Feb 15, 2018 at 1:00 PM, Andres Freund <andres@anarazel.de> wrote: > Hm, I'm not quite convinced by this. Seems to make testing a bunch of > codepaths harder. I think it's fine to say that pg doesn't work > correctly with them disabled though. I'm not sure I understand this. Do you mean we should just add a disclaimer to the documentation? As long as dynamic_shared_memory_type=none is a supported configuration, we can't use DSM for anything critical, but there's a desire to use it in things like AV and the stats collector, which are pretty critical. Instead of removing it, we could do something like this: <note> If you configure dynamic_shared_memory_type=none, parallel query will be disabled. Also, the stats collector will fail to run, so there will be no statistics to trigger autovacuum, and your database will become horribly bloated. Actually, that would be true anyway even if the stats collector somehow managed to work, because autovacuum itself will also fail. Moreover, because we've decide that you really have to have dynamic shared memory, there may probably be other things that also fail, too, either now or in the future. Basically, if you configure dynamic_shared_memory_type=none, expect a swift and painful death. </note> ...but I'm not sure that's really a great option. It seems like leaving user-visible knobs lying around that break the entire world is a bad plan. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Feb 15, 2018 at 1:00 PM, Andres Freund <andres@anarazel.de> wrote:
>> Hm, I'm not quite convinced by this. Seems to make testing a bunch of
>> codepaths harder. I think it's fine to say that pg doesn't work
>> correctly with them disabled though.
> I'm not sure I understand this. Do you mean we should just add a
> disclaimer to the documentation?
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.
regards, tom lane
On 2018-02-27 14:41:47 -0500, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Thu, Feb 15, 2018 at 1:00 PM, Andres Freund <andres@anarazel.de> wrote: > >> Hm, I'm not quite convinced by this. Seems to make testing a bunch of > >> codepaths harder. I think it's fine to say that pg doesn't work > >> correctly with them disabled though. > > > I'm not sure I understand this. Do you mean we should just add a > > disclaimer to the documentation? > > 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. Greetings, Andres Freund
Andres Freund <andres@anarazel.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.
regards, tom lane
On Tue, Feb 27, 2018 at 02:00:36PM -0500, Robert Haas wrote: > I think the two issues you are pointing out are the same issue -- > namely, if initdb probes for a max_connections setting with an invalid > DSM implementation configured, it will fail the test for every value > of max_connections and thus select the lowest possible value. The > solution to this is presumably just to make sure we choose the DSM > implementation before we do the max_connections probing so that we can > pass through the correct value there, which I think is what your patch > does. Yes, that's what the thing does. It moves the routine to find the DSM implementation before computing max_connections. -- Michael
Attachment
On Tue, Feb 27, 2018 at 02:53:55PM -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.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. dynamic_shared_memory_type is used in six code paths where it is aimed at doing sanity checks: - Mute DSM initialization at postmaster startup. - Mute cleanup of previous DSM segments from a past postmaster. - Block backend startup if there is no DSM. - Mute parallel query in planner. - Mute parallel query for parallel btree builds. - Mute creation of parallel contexts in executor. 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. -- Michael
Attachment
On Tue, Feb 27, 2018 at 2:50 PM, Andres Freund <andres@anarazel.de> wrote: > 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. Well, it's not very hard to just hack the code to make dsm_create() always fail, or fail X% of the time, if you're so inclined. I agree that -c dynamic_shared_memory_type=none is a little more convenient than sticking something like that into the code, but I don't think it's sufficiently more convenient to justify keeping an option we don't otherwise want. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Feb 27, 2018 at 2:50 PM, Andres Freund <andres@anarazel.de> wrote:
>> 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.
> Well, it's not very hard to just hack the code to make dsm_create()
> always fail, or fail X% of the time, if you're so inclined. I agree
> that -c dynamic_shared_memory_type=none is a little more convenient
> than sticking something like that into the code, but I don't think
> it's sufficiently more convenient to justify keeping an option we
> don't otherwise want.
I'd be in favor of having some normally-ifdef'd-out facility for causing
failures of this kind. (As I mentioned upthread, I do not think "always
fail" is sufficient.) That's very different from having a user-visible
option, though. We don't expose a GUC that enables CLOBBER_FREED_MEMORY,
to take a relevant example.
regards, tom lane
On Tue, Feb 27, 2018 at 11:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'd be in favor of having some normally-ifdef'd-out facility for causing > failures of this kind. (As I mentioned upthread, I do not think "always > fail" is sufficient.) That's very different from having a user-visible > option, though. We don't expose a GUC that enables CLOBBER_FREED_MEMORY, > to take a relevant example. Sure. Feel free to propose something you like. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
At Wed, 28 Feb 2018 10:08:59 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180228010859.GB1476@paquier.xyz>
> On Tue, Feb 27, 2018 at 02:53:55PM -0500, Tom Lane wrote:
> > Andres Freund <andres@anarazel.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
From 0672b1d80a08b0fb0a33a26aad8b1cc77bd83083 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 | 9 ++++-----
8 files changed, 22 insertions(+), 44 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3a8fc7d803..932ce11a58 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1602,9 +1602,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 9d4efc0f8f..4d27241502 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 14b7becf3e..c4f14391a8 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 &&
@@ -5867,8 +5866,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 67e76b98fe..8239903210 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 39272925fb..a3a678e4c1 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 1f7f2aaa54..5896d99ae1 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 0e5730f7c5..2ed1d686b3 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -14,11 +14,10 @@
#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
-#define DSM_IMPL_MMAP 4
+#define DSM_IMPL_POSIX 0
+#define DSM_IMPL_SYSV 1
+#define DSM_IMPL_WINDOWS 2
+#define DSM_IMPL_MMAP 3
/*
* Determine which dynamic shared memory implementations will be supported
--
2.16.2
Hello.
# Subject is fixed.
I think this kind of thing is sutable for doing at the beginning
of a dev cycle.
Since it has been a long time since DSM was introduced and it is
turned on by default and I believe no one turns it off on
purpose. I'd like to propose to get rid of the choice of none.
The motive of this is there's a case where fallback path is
large(?) but scarcely expected to be used.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 3322194ecf9c9dc02f1f96f71abc421cd65ff1eb 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 | 9 ++++-----
8 files changed, 22 insertions(+), 44 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7bfbc87109..68ca92d591 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/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 1f09fb6e6a..86b7b74129 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 &&
@@ -6046,8 +6045,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..0233fd4edc 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -984,6 +984,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);
@@ -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..2ed1d686b3 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -14,11 +14,10 @@
#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
-#define DSM_IMPL_MMAP 4
+#define DSM_IMPL_POSIX 0
+#define DSM_IMPL_SYSV 1
+#define DSM_IMPL_WINDOWS 2
+#define DSM_IMPL_MMAP 3
/*
* Determine which dynamic shared memory implementations will be supported
--
2.16.3
On 26.06.18 09:10, Kyotaro HORIGUCHI wrote:
> --- a/src/bin/initdb/initdb.c
> +++ b/src/bin/initdb/initdb.c
> @@ -984,6 +984,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);
>
I don't understand that comment. initdb does test whether dsm=posix
works. What more were you hoping for?
> @@ -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)
We could perhaps avoid some variability here by running the
bootstrapping runs in initdb using hardcoded dsm settings of
"sysv"/"windows".
> --- a/src/include/storage/dsm_impl.h
> +++ b/src/include/storage/dsm_impl.h
> @@ -14,11 +14,10 @@
> #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
> -#define DSM_IMPL_MMAP 4
> +#define DSM_IMPL_POSIX 0
> +#define DSM_IMPL_SYSV 1
> +#define DSM_IMPL_WINDOWS 2
> +#define DSM_IMPL_MMAP 3
I would avoid renumbering here. It was kind of sensible to have NONE =
0, so I'd keep the non-NONE ones as non-zero.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jul 06, 2018 at 11:08:02PM +0200, Peter Eisentraut wrote: > On 26.06.18 09:10, Kyotaro HORIGUCHI wrote: >> --- a/src/include/storage/dsm_impl.h >> +++ b/src/include/storage/dsm_impl.h >> @@ -14,11 +14,10 @@ >> #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 >> -#define DSM_IMPL_MMAP 4 >> +#define DSM_IMPL_POSIX 0 >> +#define DSM_IMPL_SYSV 1 >> +#define DSM_IMPL_WINDOWS 2 >> +#define DSM_IMPL_MMAP 3 > > I would avoid renumbering here. It was kind of sensible to have NONE = > 0, so I'd keep the non-NONE ones as non-zero. +1. -- Michael
Attachment
At Mon, 9 Jul 2018 10:49:19 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180709014919.GH1467@paquier.xyz>
> On Fri, Jul 06, 2018 at 11:08:02PM +0200, Peter Eisentraut wrote:
> > On 26.06.18 09:10, Kyotaro HORIGUCHI wrote:
> > I would avoid renumbering here. It was kind of sensible to have NONE =
> > 0, so I'd keep the non-NONE ones as non-zero.
>
> +1.
It is the result following your suggestion (;_;)..
Anyway, I'm willing to follow that :p
====
Thank you for the comments.
At Fri, 6 Jul 2018 23:08:02 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in
<07bdeea7-e11c-ab13-9aa1-999e13ce2e92@2ndquadrant.com>
> + /*
> + * 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.
> + */
..
> I don't understand that comment. initdb does test whether dsm=posix
> works. What more were you hoping for?
It is mentioning the fact that the test is performed *after* the
probing, and without '-c dynamic_sh..type=none', the
bootstrapping postgres uses hard-coded DSM implementation but it
may be unworkable. So we must spcify the workable one on probing
after losing the choice 'none'.
I revised the comment as below.
| * The bootstrapping postgresql may use unworkable DSM
| * implementation by default. Identify a workable one now and
| * specify it in probing steps below.
> We could perhaps avoid some variability here by running the
> bootstrapping runs in initdb using hardcoded dsm settings of
> "sysv"/"windows".
I agree. Anyway the server won't start with any configuration on
which initdb using "sysv" or "windows" fails on probing. However,
since initdb is already finding the most preferable implement for
dynamic shared memory, I think that there's no reason not to use
it.
About removing "none", the only way to choose dsm_type=none on
servers in production is rewriting the configuration file in the
generated DB cluster. Even in the case, they are using at least
sysv (or may be using POSIX) shared memory and what is required
after this patch is at most just(!) increasing shmall/shmmax
settings as required for mandatory features using DSM (nothing
yet).
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)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 444dd293e359c4165778fcefdd3b9686ff539b88 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 +++---
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 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/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
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>. -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company
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
committed On 10.07.18 08:49, Kyotaro HORIGUCHI wrote: > 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. > -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hello. At Tue, 10 Jul 2018 18:52:54 +0200, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <fda5aa38-63d6-6365-4e0b-aada93957b16@2ndquadrant.com> > committed Thank you for committing this! regards. -- Kyotaro Horiguchi NTT Open Source Software Center