Re: Interval for launching the table sync worker

On 13/04/17 13:01, Kyotaro HORIGUCHI wrote:
> Ouch! I replied to wrong mail.
>
> At Thu, 13 Apr 2017 19:55:04 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20170413(dot)195504(dot)89348773(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
>> I confused sync and apply workers.
>> sync worker failure at start causes immediate retries.
>>
>> At Thu, 13 Apr 2017 11:53:27 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in <CAD21AoCR6eHgk0vaHShjO4Bre_VDKjHUbL9EuWHaUgRPSPPyVQ(at)mail(dot)gmail(dot)com>
>>> On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut
>>> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>>>> On 4/12/17 00:48, Masahiko Sawada wrote:
>>>>> On Wed, Apr 12, 2017 at 1:28 PM, Peter Eisentraut
>>>>>> Perhaps instead of a global last_start_time, we store a per relation
>>>>>> last_start_time in SubscriptionRelState?
>>>>>
>>>>> I was thinking the same. But a problem is that the list of
>>>>> SubscriptionRelState is refreshed whenever the syncing table state
>>>>> becomes invalid (table_state_valid = false). I guess we need to
>>>>> improve these logic including GetSubscriptionNotReadyRelations().
>>>>
>>>> The table states are invalidated on a syscache callback from
>>>> pg_subscription_rel, which happens roughly speaking when a table
>>>> finishes the initial sync. So if we're worried about failing tablesync
>>>> workers relaunching to quickly, this would only be a problem if a
>>>> tablesync of another table finishes right in that restart window. That
>>>> doesn't seem a terrible issue to me.
>>>>
>>>
>>> I think the table states are invalidated whenever the table sync
>>> worker starts, because the table sync worker updates its status of
>>> pg_subscription_rel and commits it before starting actual copy. So we
>>> cannot rely on that. I thought we can store last_start_time into
>>> pg_subscription_rel but it might be overkill. I'm now thinking to
>>> change GetSubscriptionNotReadyRealtions so that last_start_time in
>>> SubscriptionRelState is taken over to new list.
>
> The right target of "This" below is found at the following URL.
>
> https://www.postgresql.org/message-id/CAD21AoBt_XUdppddFak661_LBM2t3CfK52aLKHG%2Bekd7SkzLmg%40mail.gmail.com
>
>> This resolves the problem but, if I understand correctly, the
>> many pallocs in process_syncing_tables_for_apply() is working on
>> ApplyContext and the context is reset before the next visit here
>> (in LogicalRepApplyLoop).
>>
>> Although this is not a problem of this patch, this is a problem
>> generally.

Huh? We explicitly switch to CacheMemoryContext before pallocing
anything that should survive long term.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services