Обсуждение: libpq options
The following documentation comment has been logged on the website: Page: https://www.postgresql.org/docs/9.6/static/libpq-connect.html Description: Hi everyone, The documentation refers the libpq connection option "replication" here in "Streaming Replication Protocol" page; https://www.postgresql.org/docs/current/static/protocol-replication.html However the option "replication" is not clarified (or even listed) in libpq options page. https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PARAMKEYWORDS I searched the source and find that pg_conn struct has this struct pg_conn { ... char *replication; /* connect as the replication standby? */ ... } Adding something simple like above and referring back to one of replication pages would be good I think. thanks
On Wed, Nov 22, 2017 at 10:27 PM, <sahapasci@gmail.com> wrote: > The following documentation comment has been logged on the website: > > Page: https://www.postgresql.org/docs/9.6/static/libpq-connect.html > Description: > > The documentation refers the libpq connection option "replication" here in > "Streaming Replication Protocol" page; > > https://www.postgresql.org/docs/current/static/protocol-replication.html > > However the option "replication" is not clarified (or even listed) in libpq > options page. > > https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PARAMKEYWORDS > > I searched the source and find that pg_conn struct has this > > struct pg_conn > { > ... > char *replication; /* connect as the replication standby? */ > ... > } > > Adding something simple like above and referring back to one of replication > pages would be good I think. Yeah, it is mainly a developer option which is why I guess it is not documented. Like you, I think it should be added as part of the connection parameter, and mentioned it a couple of days back: https://www.postgresql.org/message-id/CAB7nPqQAtKfG3H%2BuK11JNivtJtZYE9yVCrPuejRMjp8tUDe0nQ%40mail.gmail.com -- Michael
On Wed, Nov 22, 2017 at 11:36 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > Yeah, it is mainly a developer option which is why I guess it is not > documented. Like you, I think it should be added as part of the > connection parameter, and mentioned it a couple of days back: > https://www.postgresql.org/message-id/CAB7nPqQAtKfG3H%2BuK11JNivtJtZYE9yVCrPuejRMjp8tUDe0nQ%40mail.gmail.com Attached is a patch as an attempt to bring together the best of both worlds. The idea is to move the description of how the connection parameter replication works from the replication protocol page into the section of libpq dedicated to connection parameters, and add links between both sections. Thoughts? -- Michael
Вложения
I think this solves the consistency issue that i am talking about. Well, i
am just looking from documentation user point of view.
If it's developer only option and shouldn't be documented then maybe adding
a 'IDENTIFY_SYSTEM' parameter to pg_receivexlog is more appropriate.
like this pg_receivexlog ... --identify-system
On Sat, Nov 25, 2017 at 1:05 PM, Michael Paquier
wrote:
> On Wed, Nov 22, 2017 at 11:36 PM, Michael Paquier
> wrote:
> > Yeah, it is mainly a developer option which is why I guess it is not
> > documented. Like you, I think it should be added as part of the
> > connection parameter, and mentioned it a couple of days back:
> > https://www.postgresql.org/message-id/CAB7nPqQAtKfG3H%
> 2BuK11JNivtJtZYE9yVCrPuejRMjp8tUDe0nQ%40mail.gmail.com
>
> Attached is a patch as an attempt to bring together the best of both
> worlds. The idea is to move the description of how the connection
> parameter replication works from the replication protocol page into
> the section of libpq dedicated to connection parameters, and add links
> between both sections.
>
> Thoughts?
> --
> Michael
>
--
Şahap Aşçı
On Tue, Nov 28, 2017 at 5:00 PM, Şahap Aşçı <sahapasci@gmail.com> wrote: > I think this solves the consistency issue that i am talking about. Well, i am just looking from documentation user pointof view. Thanks. Input is always welcome. I have added an entry in the commit fest app so as this is not lost: https://commitfest.postgresql.org/16/1387/ -- Michael
On 11/28/2017 09:21 AM, Michael Paquier wrote: > On Tue, Nov 28, 2017 at 5:00 PM, Şahap Aşçı <sahapasci@gmail.com> wrote: >> I think this solves the consistency issue that i am talking about. Well, i am just looking from documentation user pointof view. > > Thanks. Input is always welcome. I have added an entry in the commit > fest app so as this is not lost: > https://commitfest.postgresql.org/16/1387/ I approve of this change. Ready for Committer. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 2017-11-25 19:05:54 +0900, Michael Paquier wrote: > + <varlistentry id="libpq-connect-replication" xreflabel="replication"> > + <term><literal>replication</literal></term> > + <listitem> > + <para> > + This option determines if a backend should use the replication > + protocol. "should"? The backend will, and the client will have to do so as well. > A Boolean value of <literal>true</literal> tells the backend > + to go into walsender mode, wherein a small set of replication commands > + can be issued instead of SQL statements. This actually is wrong now I think. Petr? Greetings, Andres Freund
On Thu, Mar 01, 2018 at 01:35:54AM -0800, Andres Freund wrote: > On 2017-11-25 19:05:54 +0900, Michael Paquier wrote: >> A Boolean value of <literal>true</literal> tells the backend >> + to go into walsender mode, wherein a small set of replication commands >> + can be issued instead of SQL statements. > > This actually is wrong now I think. Petr? On more or less HEAD: $ psql -X -d "replication=1 dbname=postgres" postgres=# create table aa (a int); ERROR: cannot execute SQL commands in WAL sender for physical replication $ psql -X -d "replication=database dbname=postgres" postgres=# create table aa (a int); CREATE TABLE So one needs to use replication=database in order to be able to issue normal SQL statements, while replication=true enforces physical replication where this cannot happen (no connection to a specified database). -- Michael
Вложения
On 01/03/18 10:35, Andres Freund wrote: > On 2017-11-25 19:05:54 +0900, Michael Paquier wrote: >> + <varlistentry id="libpq-connect-replication" xreflabel="replication"> >> + <term><literal>replication</literal></term> >> + <listitem> >> + <para> >> + This option determines if a backend should use the replication >> + protocol. > > "should"? The backend will, and the client will have to do so as well. > I think so as well. > >> A Boolean value of <literal>true</literal> tells the backend >> + to go into walsender mode, wherein a small set of replication commands >> + can be issued instead of SQL statements. > > This actually is wrong now I think. Petr? > If the replication is true, we don't have database connection so only replication commands can be executed. SQL commands work when replication is set to database. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Mar 2, 2018 at 2:50 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Mar 01, 2018 at 01:35:54AM -0800, Andres Freund wrote:
> On 2017-11-25 19:05:54 +0900, Michael Paquier wrote:
>> A Boolean value of <literal>true</literal> tells the backend
>> + to go into walsender mode, wherein a small set of replication commands
>> + can be issued instead of SQL statements.
>
> This actually is wrong now I think. Petr?
On more or less HEAD:
$ psql -X -d "replication=1 dbname=postgres"
postgres=# create table aa (a int);
ERROR: cannot execute SQL commands in WAL sender for physical replication
$ psql -X -d "replication=database dbname=postgres"
postgres=# create table aa (a int);
CREATE TABLE
So one needs to use replication=database in order to be able to issue
normal SQL statements, while replication=true enforces physical
replication where this cannot happen (no connection to a specified
database).
To nitpick:
+ protocol. A Boolean value of <literal>true</literal> tells the backend
We don't really have boolean values here, do we? It's just the string true that's treated as a boolean by the backend. It just sounds really weird to me when written that way. Particularly since the next sentence talks about passing "database" as the same thing.
I know that's pretty much nitpicking, but I want to make sure I haven't actually missed/forgotten how some part of that one is handled..
It also talks separately about "going into walsender mode" (=physical replication) and "instructs the walsender to connect to the database". I think that's a bit confusing. I suggest just calling it "physical replication mode" and "logical replication mode", and not bother mentioning walsender since that's quite internal.
On Fri, Mar 02, 2018 at 12:58:50PM +0100, Magnus Hagander wrote: > To nitpick: > > + protocol. A Boolean value of <literal>true</literal> tells the > backend > > We don't really have boolean values here, do we? It's just the string true > that's treated as a boolean by the backend. It just sounds really weird to > me when written that way. Particularly since the next sentence talks about > passing "database" as the same thing. > > I know that's pretty much nitpicking, but I want to make sure I haven't > actually missed/forgotten how some part of that one is handled.. Yes, that's indeed a bit inconsistent with the other parameters able to use boolean-like parameters, like sslcompression or sslmode (deprecated in favor requiressl), still those two ones are frontend-only parameters, so they are just able to use "1" or "0". Replication is part of the startup packet which uses parse_bool() so valid values can be true, false, yes, no, on, off, 1 or 0. And it is even possible to use upper-case characters. So why not documenting all that precisely then? > It also talks separately about "going into walsender mode" (=physical > replication) and "instructs the walsender to connect to the database". I > think that's a bit confusing. I suggest just calling it "physical > replication mode" and "logical replication mode", and not bother mentioning > walsender since that's quite internal. Indeed, this term is pretty spread across the documentation as well. I have taken into account this feedback, and created the new version attached, for a v2. Thoughts? -- Michael
Вложения
On 3/4/18 23:44, Michael Paquier wrote: > I have taken into account this feedback, and created the new version > attached, for a v2. I didn't like so much shrinking down the protocol section. I have often wanted *more* detail there, not less. So I combined your patch for the libpq chapter and added a bit more in the protocol chapter as well. Committed. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Mar 06, 2018 at 09:05:38PM -0500, Peter Eisentraut wrote: > I didn't like so much shrinking down the protocol section. I have often > wanted *more* detail there, not less. So I combined your patch for the > libpq chapter and added a bit more in the protocol chapter as well. > > Committed. Thanks for the commit, my idea was to limit duplication between both sections. If things get changed at some point, this avoids parts of the documentation to rot. <para> -The commands accepted in walsender mode are: +Replication commands are logged in the server log when +<xref linkend="guc-log-replication-commands"/> is enabled. +</para> + +<para> +The commands accepted in replication mode are: Good addition here. -- Michael