Re: [HACKERS] Interval for launching the table sync worker

On 4/27/17 21:20, Masahiko Sawada wrote: > Isn't it better to use != NIL instead as follows? > >else if (table_state != NIL && last_start_times) I'm not a fan of that in general, and it doesn't really add any clarity here. -- Peter Eisentraut http://www.2ndQuadrant.com/

Re: [HACKERS] Interval for launching the table sync worker

On 4/27/17 15:33, Petr Jelinek wrote: > On 27/04/17 21:00, Peter Eisentraut wrote: >> On 4/27/17 06:47, Petr Jelinek wrote: >>> One thing I am missing in your patch however is cleanup of entries for >>> relations that finished sync. I wonder if it would be enough to just >>> destroy the hash when

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 28, 2017 at 5:26 PM, Kyotaro HORIGUCHI wrote: > At Fri, 28 Apr 2017 10:20:48 +0900, Masahiko Sawada > wrote in

Re: [HACKERS] Interval for launching the table sync worker

At Fri, 28 Apr 2017 10:20:48 +0900, Masahiko Sawada wrote in

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 28, 2017 at 4:00 AM, Peter Eisentraut wrote: > On 4/27/17 06:47, Petr Jelinek wrote: >> One thing I am missing in your patch however is cleanup of entries for >> relations that finished sync. I wonder if it would be enough to just >> destroy the hash

Re: [HACKERS] Interval for launching the table sync worker

On 27/04/17 21:00, Peter Eisentraut wrote: > On 4/27/17 06:47, Petr Jelinek wrote: >> One thing I am missing in your patch however is cleanup of entries for >> relations that finished sync. I wonder if it would be enough to just >> destroy the hash when we get to empty list. > > I had omitted

Re: [HACKERS] Interval for launching the table sync worker

On 4/27/17 06:47, Petr Jelinek wrote: > One thing I am missing in your patch however is cleanup of entries for > relations that finished sync. I wonder if it would be enough to just > destroy the hash when we get to empty list. I had omitted that because the amount of memory "leaked" is not much,

Re: [HACKERS] Interval for launching the table sync worker

On 25/04/17 19:54, Peter Eisentraut wrote: > I feel it's getting a bit late for reworkings of this extent, also > considering the marginal nature of the problem we are trying to fix. My > patch from April 18 is very localized and gets the job done. > > I think this is still a good direction to

Re: [HACKERS] Interval for launching the table sync worker

On 4/20/17 15:36, Peter Eisentraut wrote: > On 4/19/17 23:02, Noah Misch wrote: >> This PostgreSQL 10 open item is past due for your status update. Kindly send >> a status update within 24 hours, and include a date for your subsequent >> status >> update. Refer to the policy on open item

Re: [HACKERS] Interval for launching the table sync worker

I feel it's getting a bit late for reworkings of this extent, also considering the marginal nature of the problem we are trying to fix. My patch from April 18 is very localized and gets the job done. I think this is still a good direction to investigate, but if we have to extend the hash table

Re: [HACKERS] Interval for launching the table sync worker

On 4/21/17 09:59, Petr Jelinek wrote: > Rereading the code again, it's actually not bug as we update the rstate > to what syncworker says, but it's obviously confusing so probably still > worth to commit that. We don't have the syncworker->relmutex at that point, so it's probably better to read

Re: [HACKERS] Interval for launching the table sync worker

Hello, At Tue, 25 Apr 2017 00:52:09 +0900, Masahiko Sawada wrote in > + elog(ERROR, "this hash hashtable \"%s\" is not frozen", > + hashp->tabname); > > Maybe the error message should be "this hashtable

Re: [HACKERS] Interval for launching the table sync worker

On Tue, Apr 25, 2017 at 1:42 AM, Petr Jelinek wrote: > On 24/04/17 17:52, Masahiko Sawada wrote: >> On Mon, Apr 24, 2017 at 4:41 PM, Kyotaro HORIGUCHI >> wrote: >> + /* >> + * Remove entries no longer necessary. The flag signals

Re: [HACKERS] Interval for launching the table sync worker

On 24/04/17 17:52, Masahiko Sawada wrote: > On Mon, Apr 24, 2017 at 4:41 PM, Kyotaro HORIGUCHI > wrote: > + /* > + * Remove entries no longer necessary. The flag signals nothing if > + * subrel_local_state is not updated above. We can remove entries in > + *

Re: [HACKERS] Interval for launching the table sync worker

On Mon, Apr 24, 2017 at 4:41 PM, Kyotaro HORIGUCHI wrote: > Hello, > > At Sun, 23 Apr 2017 00:51:52 +0900, Masahiko Sawada > wrote in >> On Fri, Apr 21, 2017 at 11:19 PM,

Re: [HACKERS] Interval for launching the table sync worker

Hello, At Sun, 23 Apr 2017 00:51:52 +0900, Masahiko Sawada wrote in > On Fri, Apr 21, 2017 at 11:19 PM, Masahiko Sawada > wrote: > > On Fri, Apr 21, 2017 at 5:33 PM, Kyotaro

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 21, 2017 at 11:19 PM, Masahiko Sawada wrote: > On Fri, Apr 21, 2017 at 5:33 PM, Kyotaro HORIGUCHI > wrote: >> Hello, >> >> At Thu, 20 Apr 2017 13:21:14 +0900, Masahiko Sawada >> wrote in

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 21, 2017 at 5:33 PM, Kyotaro HORIGUCHI wrote: > Hello, > > At Thu, 20 Apr 2017 13:21:14 +0900, Masahiko Sawada > wrote in

Re: [HACKERS] Interval for launching the table sync worker

On 21/04/17 10:33, Kyotaro HORIGUCHI wrote: > Hello, > > At Thu, 20 Apr 2017 13:21:14 +0900, Masahiko Sawada > wrote in

Re: [HACKERS] Interval for launching the table sync worker

On 21/04/17 04:38, Masahiko Sawada wrote: > On Thu, Apr 20, 2017 at 8:43 PM, Petr Jelinek > wrote: >> On 20/04/17 06:21, Masahiko Sawada wrote: >>> On Thu, Apr 20, 2017 at 12:30 AM, Petr Jelinek >>> wrote: On 19/04/17 15:57,

Re: [HACKERS] Interval for launching the table sync worker

Hello, At Thu, 20 Apr 2017 13:21:14 +0900, Masahiko Sawada wrote in

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 20, 2017 at 8:43 PM, Petr Jelinek wrote: > On 20/04/17 06:21, Masahiko Sawada wrote: >> On Thu, Apr 20, 2017 at 12:30 AM, Petr Jelinek >> wrote: >>> On 19/04/17 15:57, Masahiko Sawada wrote: On Wed, Apr 19, 2017 at

Re: [HACKERS] Interval for launching the table sync worker

On 4/19/17 23:02, Noah Misch wrote: > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: >

Re: [HACKERS] Interval for launching the table sync worker

On 20/04/17 06:21, Masahiko Sawada wrote: > On Thu, Apr 20, 2017 at 12:30 AM, Petr Jelinek > wrote: >> On 19/04/17 15:57, Masahiko Sawada wrote: >>> On Wed, Apr 19, 2017 at 10:07 PM, Petr Jelinek >>> wrote: On 19/04/17 14:42,

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 20, 2017 at 12:30 AM, Petr Jelinek wrote: > On 19/04/17 15:57, Masahiko Sawada wrote: >> On Wed, Apr 19, 2017 at 10:07 PM, Petr Jelinek >> wrote: >>> On 19/04/17 14:42, Masahiko Sawada wrote: On Wed, Apr 19, 2017 at

Re: [HACKERS] Interval for launching the table sync worker

On Sun, Apr 16, 2017 at 06:08:41AM +, Noah Misch wrote: > On Thu, Apr 06, 2017 at 11:33:22AM +0900, Masahiko Sawada wrote: > > While testing table sync worker for logical replication I noticed that > > if the table sync worker of logical replication failed to insert the > > data for whatever

Re: [HACKERS] Interval for launching the table sync worker

On 19/04/17 15:57, Masahiko Sawada wrote: > On Wed, Apr 19, 2017 at 10:07 PM, Petr Jelinek > wrote: >> On 19/04/17 14:42, Masahiko Sawada wrote: >>> On Wed, Apr 19, 2017 at 5:12 PM, Kyotaro HORIGUCHI >>> wrote: At Tue, 18 Apr

Re: [HACKERS] Interval for launching the table sync worker

On Wed, Apr 19, 2017 at 10:07 PM, Petr Jelinek wrote: > On 19/04/17 14:42, Masahiko Sawada wrote: >> On Wed, Apr 19, 2017 at 5:12 PM, Kyotaro HORIGUCHI >> wrote: >>> At Tue, 18 Apr 2017 18:40:56 +0200, Petr Jelinek >>>

Re: [HACKERS] Interval for launching the table sync worker

On 19/04/17 14:42, Masahiko Sawada wrote: > On Wed, Apr 19, 2017 at 5:12 PM, Kyotaro HORIGUCHI > wrote: >> At Tue, 18 Apr 2017 18:40:56 +0200, Petr Jelinek >> wrote in >> >>>

Re: [HACKERS] Interval for launching the table sync worker

On Wed, Apr 19, 2017 at 5:12 PM, Kyotaro HORIGUCHI wrote: > At Tue, 18 Apr 2017 18:40:56 +0200, Petr Jelinek > wrote in > >> On 18/04/17 18:14, Peter Eisentraut wrote: >> > On

Re: [HACKERS] Interval for launching the table sync worker

At Tue, 18 Apr 2017 18:40:56 +0200, Petr Jelinek wrote in > On 18/04/17 18:14, Peter Eisentraut wrote: > > On 4/18/17 11:59, Petr Jelinek wrote: > >> Hmm if we create hashtable for this, I'd say create hashtable

Re: [HACKERS] Interval for launching the table sync worker

On 18/04/17 18:14, Peter Eisentraut wrote: > On 4/18/17 11:59, Petr Jelinek wrote: >> Hmm if we create hashtable for this, I'd say create hashtable for the >> whole table_states then. The reason why it's list now was that it seemed >> unnecessary to have hashtable when it will be empty almost

Re: [HACKERS] Interval for launching the table sync worker

On 4/18/17 11:59, Petr Jelinek wrote: > Hmm if we create hashtable for this, I'd say create hashtable for the > whole table_states then. The reason why it's list now was that it seemed > unnecessary to have hashtable when it will be empty almost always but > there is no need to have both hashtable

Re: [HACKERS] Interval for launching the table sync worker

On Tue, Apr 18, 2017 at 11:22 PM, Peter Eisentraut wrote: > On 4/13/17 06:23, Masahiko Sawada wrote: >> Attached the latest patch. It didn't actually necessary to change >> GetSubscriptionNotReadyRelations. I just changed the logic refreshing >> the sync table

Re: [HACKERS] Interval for launching the table sync worker

On 18/04/17 16:22, Peter Eisentraut wrote: > On 4/13/17 06:23, Masahiko Sawada wrote: >> Attached the latest patch. It didn't actually necessary to change >> GetSubscriptionNotReadyRelations. I just changed the logic refreshing >> the sync table state list. > > I think this was the right

Re: [HACKERS] Interval for launching the table sync worker

On 4/13/17 06:23, Masahiko Sawada wrote: > Attached the latest patch. It didn't actually necessary to change > GetSubscriptionNotReadyRelations. I just changed the logic refreshing > the sync table state list. I think this was the right direction, but then I got worried about having a loop within

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 06, 2017 at 11:33:22AM +0900, Masahiko Sawada wrote: > While testing table sync worker for logical replication I noticed that > if the table sync worker of logical replication failed to insert the > data for whatever reason, the table sync worker process exits with > error. And then

Re: [HACKERS] Interval for launching the table sync worker

On 4/13/17 18:09, Petr Jelinek wrote: > It would have the > disadvantage that if some tables were consistently failing, no other > tables could get synchronized as the amount of workers is limited. OTOH > the approach chosen in the patch will first try all tables and only then > start rate

Re: [HACKERS] Interval for launching the table sync worker

On 4/13/17 06:23, Masahiko Sawada wrote: > Attached the latest patch. It didn't actually necessary to change > GetSubscriptionNotReadyRelations. I just changed the logic refreshing > the sync table state list. > Please give me feedback. I think this is a reasonable direction, but do we really

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 14, 2017 at 9:14 PM, Petr Jelinek wrote: > On 14/04/17 12:18, Masahiko Sawada wrote: >> On Fri, Apr 14, 2017 at 7:09 AM, Petr Jelinek >> wrote: >>> On 13/04/17 12:23, Masahiko Sawada wrote: On Thu, Apr 13, 2017 at 11:53

Re: [HACKERS] Interval for launching the table sync worker

On 14/04/17 12:18, Masahiko Sawada wrote: > On Fri, Apr 14, 2017 at 7:09 AM, Petr Jelinek > wrote: >> On 13/04/17 12:23, Masahiko Sawada wrote: >>> On Thu, Apr 13, 2017 at 11:53 AM, Masahiko Sawada >>> wrote: On Wed, Apr 12, 2017 at

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 14, 2017 at 7:09 AM, Petr Jelinek wrote: > On 13/04/17 12:23, Masahiko Sawada wrote: >> On Thu, Apr 13, 2017 at 11:53 AM, Masahiko Sawada >> wrote: >>> On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut >>>

Re: [HACKERS] Interval for launching the table sync worker

At Thu, 13 Apr 2017 20:02:33 +0200, Petr Jelinek wrote in > >> Although this is not a problem of this patch, this is a problem > >> generally. > > Huh? We explicitly switch to CacheMemoryContext before

Re: [HACKERS] Interval for launching the table sync worker

On 13/04/17 12:23, Masahiko Sawada wrote: > On Thu, Apr 13, 2017 at 11:53 AM, Masahiko Sawada > wrote: >> On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut >> wrote: >>> On 4/12/17 00:48, Masahiko Sawada wrote: On Wed, Apr 12, 2017

Re: [HACKERS] 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 > wrote in > <20170413.195504.89348773.horiguchi.kyot...@lab.ntt.co.jp> >> I confused sync and apply

Re: [HACKERS] Interval for launching the table sync worker

Ouch! I replied to wrong mail. At Thu, 13 Apr 2017 19:55:04 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20170413.195504.89348773.horiguchi.kyot...@lab.ntt.co.jp> > I confused sync and apply workers. > sync worker failure at start causes immediate

Re: [HACKERS] Interval for launching the table sync worker

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 wrote in > On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 13, 2017 at 11:53 AM, Masahiko Sawada wrote: > On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut > wrote: >> On 4/12/17 00:48, Masahiko Sawada wrote: >>> On Wed, Apr 12, 2017 at 1:28 PM, Peter Eisentraut Perhaps instead

Re: [HACKERS] Interval for launching the table sync worker

On Wed, Apr 12, 2017 at 11:46 PM, Peter Eisentraut 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

Re: [HACKERS] Interval for launching the table sync worker

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

Re: [HACKERS] Interval for launching the table sync worker

On Wed, Apr 12, 2017 at 1:28 PM, Peter Eisentraut wrote: > On 4/10/17 08:10, Petr Jelinek wrote: >> I don't think solution is quite this simple. This will cause all table >> sync workers to be delayed which means concurrency will suffer and the >> initial sync of

Re: [HACKERS] Interval for launching the table sync worker

On 4/10/17 08:10, Petr Jelinek wrote: > I don't think solution is quite this simple. This will cause all table > sync workers to be delayed which means concurrency will suffer and the > initial sync of all tables will take much longer especially if there is > little data. We need a way to either

Re: [HACKERS] Interval for launching the table sync worker

On 10/04/17 11:02, Masahiko Sawada wrote: > On Sun, Apr 9, 2017 at 6:25 PM, Masahiko Sawada wrote: >> On Sat, Apr 8, 2017 at 8:13 AM, Peter Eisentraut >> wrote: >>> On 4/7/17 01:10, Masahiko Sawada wrote: It's not critical but it

Re: [HACKERS] Interval for launching the table sync worker

On Sun, Apr 9, 2017 at 6:25 PM, Masahiko Sawada wrote: > On Sat, Apr 8, 2017 at 8:13 AM, Peter Eisentraut > wrote: >> On 4/7/17 01:10, Masahiko Sawada wrote: >>> It's not critical but it could be problem. So I thought we should fix >>> it

Re: [HACKERS] Interval for launching the table sync worker

On Sat, Apr 8, 2017 at 8:13 AM, Peter Eisentraut wrote: > On 4/7/17 01:10, Masahiko Sawada wrote: >> It's not critical but it could be problem. So I thought we should fix >> it before the PostgreSQL 10 release. If it's not appropriate as an >> open item I'll

Re: [HACKERS] Interval for launching the table sync worker

On 4/7/17 01:10, Masahiko Sawada wrote: > It's not critical but it could be problem. So I thought we should fix > it before the PostgreSQL 10 release. If it's not appropriate as an > open item I'll remove it. You wrote that you "sent" a patch, but I don't see a patch anywhere. I think a

Re: [HACKERS] Interval for launching the table sync worker

On Fri, Apr 7, 2017 at 1:23 PM, Kyotaro HORIGUCHI wrote: > At Thu, 6 Apr 2017 18:42:37 +0200, Petr Jelinek > wrote in > <8c7afb37-be73-c6bd-80bc-e87522f04...@2ndquadrant.com> >> On 06/04/17 16:44, Masahiko Sawada wrote: >> > On

Re: [HACKERS] Interval for launching the table sync worker

At Thu, 6 Apr 2017 18:42:37 +0200, Petr Jelinek wrote in <8c7afb37-be73-c6bd-80bc-e87522f04...@2ndquadrant.com> > On 06/04/17 16:44, Masahiko Sawada wrote: > > On Thu, Apr 6, 2017 at 9:06 PM, Kyotaro HORIGUCHI > > wrote: > >>> I

Re: [HACKERS] Interval for launching the table sync worker

On 06/04/17 16:44, Masahiko Sawada wrote: > On Thu, Apr 6, 2017 at 9:06 PM, Kyotaro HORIGUCHI > wrote: >> At Thu, 06 Apr 2017 17:02:14 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI >> wrote in >>

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 6, 2017 at 9:06 PM, Kyotaro HORIGUCHI wrote: > At Thu, 06 Apr 2017 17:02:14 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI > wrote in > <20170406.170214.263553093.horiguchi.kyot...@lab.ntt.co.jp> >> At Thu, 6 Apr

Re: [HACKERS] Interval for launching the table sync worker

At Thu, 06 Apr 2017 17:02:14 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI wrote in <20170406.170214.263553093.horiguchi.kyot...@lab.ntt.co.jp> > At Thu, 6 Apr 2017 16:15:33 +0900, Masahiko Sawada > wrote in

Re: [HACKERS] Interval for launching the table sync worker

At Thu, 6 Apr 2017 16:15:33 +0900, Masahiko Sawada wrote in > On Thu, Apr 6, 2017 at 3:45 PM, Kyotaro HORIGUCHI > wrote: > > I was thinking the same. > > > > At Thu, 6

Re: [HACKERS] Interval for launching the table sync worker

On Thu, Apr 6, 2017 at 3:45 PM, Kyotaro HORIGUCHI wrote: > I was thinking the same. > > At Thu, 6 Apr 2017 11:33:22 +0900, Masahiko Sawada > wrote in

Re: [HACKERS] Interval for launching the table sync worker

I was thinking the same. At Thu, 6 Apr 2017 11:33:22 +0900, Masahiko Sawada wrote in