Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RDY handling on new connections #38

Merged
merged 1 commit into from
Jul 10, 2013

Conversation

jehiah
Copy link
Member

@jehiah jehiah commented Jun 27, 2013

When a new connection is added (beyond the first one) it never get's an initial RDY count (even though max_in_flight might be high enough). As a result subsequent connections may stay at RDY 0 (until backoff and recover happens at which time expected max_in_flight is distributed across connections correctly).

@jehiah
Copy link
Member Author

jehiah commented Jun 27, 2013

RFR @mreiferson

cc: @ploxiln @jphines

@mreiferson
Copy link
Member

I'm not sure what you were experiencing but I dont think this is necessary.

The first (existing) clause already handles all non-backoff cases (meaning every new connection while not in backoff will attempt to send RDY 1 up to max_in_flight).

I'm assuming then that the worker in question was already in backoff... in which case it is correct for subsequent new connections to stay at RDY 0 until recovery from backoff or redistribution (which is already set to run for those cases).

What am I missing?

@mreiferson
Copy link
Member

Also, if a connection is going to get an initial non-0 RDY count, we had previously decided to always use 1 to simplify. I don't see any reason to change this.

@jehiah
Copy link
Member Author

jehiah commented Jun 27, 2013

Hmmm you are right. It might be the case that at the time of subsequent connect finishing the first connection had acquired the full max_in_flight.

@mreiferson
Copy link
Member

If there is indeed some edge case here one idea is for initial RDY 0'd connections to enter a poll loop and try to set RDY 1 until it either is non-0 or successful.... (every 5 secs or something?)

@mreiferson
Copy link
Member

(ie it sounds possible for an atypical redistribute case where other conns "stole" all the max in flight to never get a RDY 1)

@mreiferson
Copy link
Member

see mreiferson/pynsq@4c99a96 for one possible solution

@mreiferson
Copy link
Member

pushed another commit addressing what we discussed (somewhat unrelated to this specific issue but we can just update subject/description) @jehiah

@jehiah
Copy link
Member Author

jehiah commented Jul 10, 2013

I like the last commit. It's a good clean way to spread RDY to other connections, and should sove this situation.

@jehiah
Copy link
Member Author

jehiah commented Jul 10, 2013

@mreiferson I'm good on this. any changes you want before I squash?

@mreiferson
Copy link
Member

im good

* occasionally choose a random connection for RDY when num conns > max_in_flight
* loop on a new connection when we're at RDY 0 and we cant send RDY non-0
mreiferson added a commit that referenced this pull request Jul 10, 2013
RDY handling on new connections
@mreiferson mreiferson merged commit 398cca5 into nsqio:master Jul 10, 2013
@jehiah jehiah mentioned this pull request Aug 19, 2013
5 tasks
@mreiferson mreiferson mentioned this pull request Sep 3, 2013
6 tasks
@jehiah jehiah deleted the new_connections_38 branch June 28, 2014 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants