On Mon, Mar 20, 2017 at 7:56 PM, Petr Jelinek <petr.jeli...@2ndquadrant.com> wrote: > On 18/03/17 13:31, Petr Jelinek wrote: >> On 07/03/17 06:23, Petr Jelinek wrote: >>> there has been discussion at the logical replication initial copy thread >>> [1] about making apply work with sync commit off by default for >>> performance reasons and adding option to change that per subscription. >>> >>> Here I propose patch to implement this - it adds boolean column >>> subssynccommit to pg_subscription catalog which decides if >>> synchronous_commit should be off or local for apply. And it adds >>> SYNCHRONOUS_COMMIT = boolean to the list of WITH options for CREATE and >>> ALTER SUBSCRIPTION. When nothing is specified it will set it to false. >>> >>> The patch is built on top of copy patch currently as there are conflicts >>> between the two and this helps a bit with testing of copy patch. >>> >>> [1] >>> https://www.postgresql.org/message-id/CA+TgmoY7Lk2YKArcp4O=Qu=xoor8j71mad1oteojawmuje3...@mail.gmail.com >>> >> >> I rebased this patch against recent changes and the latest version of >> copy patch. > > And another rebase after pg_dump tests commit.
+ else if (strcmp(defel->defname, "nosynchronous_commit") == 0 && synchronous_commit) + { + if (synchronous_commit_given) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + + synchronous_commit_given = true; + *synchronous_commit = !defGetBoolean(defel); + } Uh, what's this nosynchronous_commit thing? + <literal>local</literal> otherwise to <literal>false</literal>. The + default value is <literal>false</literal> independently of the default + <literal>synchronous_commit</literal> setting for the instance. This phrasing isn't very clear or accurate, IMHO. I'd say something like "The value of this parameter overrides the synchronous_commit setting. The default value is false." And I'd make the word synchronous_commit in that sentence a link to the GUC, so that it's absolutely unmistakable what we mean by "the synchronous_commit setting". /* + * We need to make new connection to new slot if slot name has changed + * so exit here as well if that's the case. + */ + if (strcmp(newsub->slotname, MySubscription->slotname) != 0) + { + ereport(LOG, + (errmsg("logical replication worker for subscription \"%s\" will " + "restart because the replication slot name was changed", + MySubscription->name))); + + walrcv_disconnect(wrconn); + proc_exit(0); + } Looks unrelated. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers