On 21/04/17 16:23, Peter Eisentraut wrote: > On 4/21/17 10:11, Petr Jelinek wrote: >> On 21/04/17 16:09, Peter Eisentraut wrote: >>> On 4/20/17 14:29, Petr Jelinek wrote: >>>> + /* Find unused worker slot. */ >>>> + if (!w->in_use) >>>> { >>>> - worker = &LogicalRepCtx->workers[slot]; >>>> - break; >>>> + worker = w; >>>> + slot = i; >>>> + } >>> >>> Doesn't this still need a break? Otherwise it always picks the last slot. >>> >> >> Yes it will pick the last slot, does that matter though, is the first >> one better somehow? >> >> We can't break because we also need to continue the counter (I think the >> issue that the counter solves is probably just theoretical, but still). > > I see. I think the code would be less confusing if we break the loop > like before and call logicalrep_sync_worker_count() separately. > >> Hmm actually, maybe the if (!w->in_use) should be if (worker == NULL && >> !w->in_use)? > > That would also do it. But it's getting a bit fiddly. >
I just wanted to avoid looping twice, especially since the garbage collecting code has to also do the loop. I guess I'll go with my original coding for this then which was to put retry label above the loop first, then try finding worker slot, if found call the logicalrep_sync_worker_count and if not found do the garbage collection and if we cleaned up something then goto retry. -- 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