Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1huniJ-0001gj-0v for pgsql-hackers@arkaria.postgresql.org; Tue, 06 Aug 2019 00:52:31 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1huniH-0003pM-6h for pgsql-hackers@arkaria.postgresql.org; Tue, 06 Aug 2019 00:52:29 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1huniG-0003pF-Rm for pgsql-hackers@lists.postgresql.org; Tue, 06 Aug 2019 00:52:28 +0000 Received: from tamriel.snowman.net ([96.255.250.162]) by makus.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1huniD-00040l-Ub for pgsql-hackers@postgresql.org; Tue, 06 Aug 2019 00:52:27 +0000 Received: by tamriel.snowman.net (Postfix, from userid 1000) id 1AD8B5F79F; Mon, 5 Aug 2019 20:52:26 -0400 (EDT) Date: Mon, 5 Aug 2019 20:52:26 -0400 From: Stephen Frost To: Tom Lane Cc: Robert Haas , Tomas Vondra , Amit Kapila , Ian Barwick , "pgsql-hackers@postgresql.org" Subject: Re: [PATCH] Stop ALTER SYSTEM from making bad assumptions Message-ID: <20190806005225.GM29202@tamriel.snowman.net> References: <20190802230011.dqokdhjptykopxxc@development> <20190805142139.GW29202@tamriel.snowman.net> <20190805172935.GK29202@tamriel.snowman.net> <30179.1565029753@sss.pgh.pa.us> <30823.1565030571@sss.pgh.pa.us> <32103.1565032053@sss.pgh.pa.us> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="wjkOZ/HuI+BCD7Ff" Content-Disposition: inline In-Reply-To: <32103.1565032053@sss.pgh.pa.us> User-Agent: Mutt/1.5.24 (2015-08-30) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --wjkOZ/HuI+BCD7Ff Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Robert Haas writes: > > On Mon, Aug 5, 2019 at 2:42 PM Tom Lane wrote: > >> I don't think we need to go on about it at great length, but it seems > >> to me that it'd be reasonable to point out that (a) you'd be well > >> advised not to touch the file while the postmaster is up, and (b) > >> last setting wins. Those things are equally true of postgresql.conf > >> of course, but I don't recall whether they're already documented. >=20 > > OK, fair enough. >=20 > Concretely, how about the attached? > (Digging around in config.sgml, I found that last-one-wins is stated, > but only in the context of one include file overriding another. > That's not *directly* a statement about what happens within a single > file, and it's in a different subsection anyway, so repeating the > info in 19.1.2 doesn't seem unreasonable.) Agreed. > diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml > index cdc30fa..f5986b2 100644 > --- a/doc/src/sgml/config.sgml > +++ b/doc/src/sgml/config.sgml > @@ -153,6 +153,8 @@ shared_buffers =3D 128MB > identifiers or numbers must be single-quoted. To embed a single > quote in a parameter value, write either two quotes (preferred) > or backslash-quote. > + If the file contains multiple entries for the same parameter, > + all but the last one are ignored. > Looking at this patch quickly but also in isolation, so I could be wrong here, but it seems like the above might be a good place to mention "duplicate entries and comments may be removed." > > @@ -185,18 +187,27 @@ shared_buffers =3D 128MB > In addition to postgresql.conf, > a PostgreSQL data directory contains a f= ile > postgresql.auto.confpostgr= esql.auto.conf, > - which has the same format as postgresql.conf b= ut should > - never be edited manually. This file holds settings provided through > - the command. This file is auto= matically > - read whenever postgresql.conf is, and its sett= ings take > - effect in the same way. Settings in postgresql.auto.conf= > - override those in postgresql.conf. > + which has the same format as postgresql.conf b= ut > + is intended to be edited automatically not manually. This file hol= ds > + settings provided through the c= ommand. > + This file is read whenever postgresql.conf is, > + and its settings take effect in the same way. Settings > + in postgresql.auto.conf override those > + in postgresql.conf. > + The above hunk looks fine. > + > + External tools might also > + modify postgresql.auto.conf, typically by appe= nding > + new settings to the end. It is not recommended to do this while the > + server is running, since a concurrent ALTER SYSTEM > + command could overwrite such changes. > Alternatively, or maybe also here, we could say "note that appending to the file as a mechanism for setting a new value by an external tool is acceptable even though it will cause duplicates- PostgreSQL will always use the last value set and other tools should as well. Duplicates and comments may be removed when rewriting the file, and parameters may be lower-cased." (istr that last bit being true too but I haven't checked lately). > > The system view > pg_file_setting= s > - can be helpful for pre-testing changes to the configuration file, o= r for > + can be helpful for pre-testing changes to the configuration files, = or for > diagnosing problems if a SIGHUP signal did= not have the > desired effects. > This hunk looks fine. Reviewing https://www.postgresql.org/docs/current/config-setting.html again, it looks reasonably comprehensive regarding the format of the file- perhaps we should link to there from the "external tools might also modify" para..? "Tool authors should review to understand the structure of postgresql.auto.conf". Thanks! Stephen --wjkOZ/HuI+BCD7Ff Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJdSM9JAAoJEO1sijiDR2RVzQoP+wU8JdlOJtQgdZlzXDOgpcVb m4lvaiwXI7eKuphUqvc45qbWJUYc7+5k0huhE8KG5SmFpKsB3qa5hKsFleO304hI v6Jh9M0NqPuwxvyS6xgKtc8IEIlijDdDAMdNOeoyEYwyARVYDEQRp0G0x1yLSOo7 UT8gCMsKynuu5uk3deKWSRw2edDj3R7pTI8bMiRF5Nw3uX9KfRdPDibKF4UArJSb h0riKaQkkbjSi+IfKnScVI0ND3OZXpmTq7462dkR7NQ1DlcEi/kaI1yjMbdwAfpp CmjAsYL7DOz0dYV43zDlWKrDrqP8FFvmidbyGZOQNZ/CTlWqxw4hnvt8LxaBNt8F cRn4Of8/savd+GwunuZEzfuBe+BsOi/TP4w7y0xp0Pk0qK2g6x3XnUr9m9uRT98j sgXU3csGAqdh42qyA6aG0SPxQySBUaR7VjVTF7fF9qLZ+SsWwMMdA3S1CMyQKviq 7dkpmYxYFIF3eI60hcoaBeubn414c7ZjoOnD4eP+7Oq9NRHGvbpNwugyU91z7g3f qxx+KhHtwrMfJAdPOnz1H74uIVioE37ortiJwUH7k8tN7yZqJvQlzNli67SPYR0+ Qub4Q8msvHf+0bgsJHORzIE3ReKHIvXPqiHDDaGyNG08/JJBpdX0T9iutomMwUMp ihibYpVnSqeJAL2jsCy4 =AXlx -----END PGP SIGNATURE----- --wjkOZ/HuI+BCD7Ff--