From: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Erik Rijkers <er(at)xs4all(dot)nl>, Steve Singer <steve(at)ssinger(dot)info>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Logical Replication WIP |
Date: | 2017-01-23 16:56:03 |
Message-ID: | 3b3130df-bcbf-3a34-06b8-022599e47659@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 23/01/17 17:19, Fujii Masao wrote:
> On Sat, Jan 21, 2017 at 1:39 AM, Petr Jelinek
> <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>> On 20/01/17 17:33, Jaime Casanova wrote:
>>> On 20 January 2017 at 11:25, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>>> On 20/01/17 17:05, Fujii Masao wrote:
>>>>> On Fri, Jan 20, 2017 at 11:08 PM, Peter Eisentraut
>>>>> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>>>>>> On 1/19/17 5:01 PM, Petr Jelinek wrote:
>>>>>>> There were some conflicting changes committed today so I rebased the
>>>>>>> patch on top of them.
>>>>>>>
>>>>>>> Other than that nothing much has changed, I removed the separate sync
>>>>>>> commit patch, included the rename patch in the patchset and fixed the
>>>>>>> bug around pg_subscription catalog reported by Erik Rijkers.
>>>>>>
>>>>>> Committed.
>>>>>
>>>>> Sorry I've not followed the discussion about logical replication at all, but
>>>>> why does logical replication launcher need to start up by default?
>>>>>
>>>>
>>>> Because running subscriptions is allowed by default. You'd need to set
>>>> max_logical_replication_workers to 0 to disable that.
>>>>
>>>
>>> surely wal_level < logical shouldn't start a logical replication
>>> launcher, and after an initdb wal_level is only replica
>>>
>>
>> Launcher is needed for subscriptions, subscriptions don't depend on
>> wal_level.
>
> But why did you enable only subscription by default while publication is
> disabled by default (i.e., wal_level != logical)? I think that it's better to
> enable both by default OR disable both by default.
>
That depends, the wal_level = logical by default was deemed to not be
worth the potential overhead in the thread about wal_level thread. There
is no such overhead associated with enabling subscription, one could say
that it's less work this way to setup whole thing. But I guess it's up
for a debate.
> While I was reading the logical rep code, I found that
> logicalrep_worker_launch returns *without* releasing LogicalRepWorkerLock
> when there is no unused worker slot. This seems a bug.
True, fix attached.
>
> /* Report this after the initial starting message for consistency. */
> if (max_replication_slots == 0)
> ereport(ERROR,
> (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
> errmsg("cannot start logical replication workers when
> max_replication_slots = 0")));
>
> logicalrep_worker_launch checks max_replication_slots as above.
> Why does it need to check that setting value in the *subscriber* side?
> Maybe I'm missing something here, but ISTM that the subscription uses
> one replication slot in *publisher* side but doesn't use in *subscriber* side.
Because replication origins are also limited by the
max_replication_slots and they are required for subscription to work (I
am not quite sure why it's the case, I guess we wanted to save GUC).
>
> * The apply worker may spawn additional workers (sync) for initial data
> * synchronization of tables.
>
> The above header comment in logical/worker.c is true?
>
Hmm not yet, there is separate patch for it in CF, I guess it got
through the cracks while rebasing.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
0001-Release-lock-on-failure-to-launch-replication-worker.patch | application/x-patch | 870 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | David Christensen | 2017-01-23 16:59:32 | Re: Online enabling of page level checksums |
Previous Message | Simon Riggs | 2017-01-23 16:53:27 | Re: Online enabling of page level checksums |