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

Reply via email to