Re: Let's remove DSM_IMPL_NONE.

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: michael(at)paquier(dot)xyz
Cc: peter(dot)eisentraut(at)2ndquadrant(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us, andres(at)anarazel(dot)de, robertmhaas(at)gmail(dot)com
Subject: Re: Let's remove DSM_IMPL_NONE.
Date: 2018-07-09 09:07:24
Message-ID: 20180709.180724.182351986.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Mon, 9 Jul 2018 10:49:19 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180709014919(dot)GH1467(at)paquier(dot)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(dot)eisentraut(at)2ndquadrant(dot)com> wrote in <07bdeea7-e11c-ab13-9aa1-999e13ce2e92(at)2ndquadrant(dot)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

Attachment Content-Type Size
v4-0001-Remove-dynamic_shared_memroy_type-none.patch text/x-patch 8.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-07-09 09:08:07 Re: Subplan result caching
Previous Message Tsunakawa, Takayuki 2018-07-09 08:29:08 RE: How can we submit code patches that implement our (pending) patents?