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.kyot...@lab.ntt.co.jp> wrote in > <20170413.195504.89348773.horiguchi.kyot...@lab.ntt.co.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.m...@gmail.com> >> wrote in <cad21aocr6ehgk0vahshjo4bre_vdkjhubl9euwhaugrpspp...@mail.gmail.com> >>> On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut >>> <peter.eisentr...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
