Re: logical replication apply to run with sync commit off by default

On Fri, Mar 24, 2017 at 11:49 PM, Petr Jelinek
<petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
> On 21/03/17 22:37, Petr Jelinek wrote:
>> On 21/03/17 18:54, Robert Haas wrote:
>>> On Mon, Mar 20, 2017 at 7:56 PM, Petr Jelinek
>>> <petr(dot)jelinek(at)2ndquadrant(dot)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=xOoR8j71MAd1OTEojAWmujE3k0w@mail.gmail.com
>>>>>>
>>>>>
>>>>> I rebased this patch against recent changes and the latest version of
>>>>> copy patch.
>>>>
>
> Rebase after table copy patch got committed.
>

This patch seems to conflict current HEAD. Please update the patch.

$ patch -p1 < 0001-Add-option-to-modify-sync-commit-per-subscription.patch
(Stripping trailing CRs from patch.)
patching file doc/src/sgml/catalogs.sgml
Hunk #1 succeeded at 6511 (offset 97 lines).
(Stripping trailing CRs from patch.)
patching file doc/src/sgml/ref/alter_subscription.sgml
(Stripping trailing CRs from patch.)
patching file doc/src/sgml/ref/create_subscription.sgml
(Stripping trailing CRs from patch.)
patching file src/backend/catalog/pg_subscription.c
(Stripping trailing CRs from patch.)
patching file src/backend/commands/subscriptioncmds.c
(Stripping trailing CRs from patch.)
patching file src/backend/replication/logical/launcher.c
(Stripping trailing CRs from patch.)
patching file src/backend/replication/logical/worker.c
Hunk #1 succeeded at 1395 (offset -15 lines).
Hunk #2 succeeded at 1468 (offset -15 lines).
(Stripping trailing CRs from patch.)
patching file src/bin/pg_dump/pg_dump.c
Hunk #1 succeeded at 3654 (offset 1 line).
Hunk #2 succeeded at 3672 (offset 1 line).
Hunk #3 succeeded at 3687 (offset 1 line).
Hunk #4 succeeded at 3705 (offset 1 line).
Hunk #5 succeeded at 3776 (offset 1 line).
(Stripping trailing CRs from patch.)
patching file src/bin/pg_dump/pg_dump.h
Hunk #1 succeeded at 612 (offset 8 lines).
(Stripping trailing CRs from patch.)
patching file src/bin/pg_dump/t/002_pg_dump.pl
(Stripping trailing CRs from patch.)
patching file src/bin/psql/describe.c
Hunk #1 succeeded at 5171 (offset 51 lines).
Hunk #2 succeeded at 5198 (offset 51 lines).
(Stripping trailing CRs from patch.)
patching file src/include/catalog/pg_subscription.h
(Stripping trailing CRs from patch.)
patching file src/test/regress/expected/subscription.out
Hunk #1 FAILED at 25.
Hunk #2 succeeded at 89 (offset 25 lines).
1 out of 2 hunks FAILED -- saving rejects to file
src/test/regress/expected/subscription.out.rej
(Stripping trailing CRs from patch.)
patching file src/test/regress/sql/subscription.sql
Hunk #1 succeeded at 66 (offset 21 lines).

-----
+ <para>
+ The value of this parameter overrides the
+ <xref linkend="guc-synchronous-commit">synchronous_commit setting.
+ The default value is <literal>false<literal>.
+ </para>

+ <entry>
+ If true, the apply for the subscription will run with
+ <literal>synchronous_commit</literal> set to <literal>local</literal>.
+ Otherwise it will have it set to <literal>false</literal>.
+ </entry>

synchronous_commit can work with false actually but I think that it's
better to use "off" instead of "false" according to the document.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center