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

nsqd: add random load balancing for Authd query #1163

Merged
merged 1 commit into from Jun 18, 2019

Conversation

shenhui0509
Copy link

@shenhui0509 shenhui0509 commented Jun 18, 2019

In our production environment

  1. we have proxies backended with nsqds
  2. auth is on
  3. proxy uses a connection pool to request on nsqd
    when there is high pub concurrency, proxy will create new connection, thus will query authd. The first authd will be overload, and the latency will increase, result in the risk of avalanche. Add a load balance for authd can ease that.
    on consumer sides, we also have a large number of consumer instances, Load balance for authd is also benefit.
    ready for review

@shenhui0509
Copy link
Author

ready for review

@andyxning
Copy link
Member

/cc @mreiferson @ploxiln We encounter this in our production environment. PTAL.

@jehiah jehiah added the perf label Jun 18, 2019
@jehiah jehiah changed the title add random load balance for query authds nsqd: add random load balancing for Authd query Jun 18, 2019
Copy link
Member

@jehiah jehiah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this looks great.

One last thing, can you rebase and squash your commits down to a single commit w/ an appropriate commit message before we merge?

@shenhui0509
Copy link
Author

👍 this looks great.

One last thing, can you rebase and squash your commits down to a single commit w/ an appropriate commit message before we merge?

resolved

@andyxning
Copy link
Member

@jehiah Thanks for reviewing! Commits are squashed. PTAL.

@jehiah jehiah merged commit 1db3903 into nsqio:master Jun 18, 2019
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

3 participants