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

New option "auto_eject_drop" #214

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mezzatto
Copy link
Contributor

@mezzatto mezzatto commented Apr 4, 2014

A boolean value that controls if auto ejected hosts should be dropped from the hash ring. If set to false, failing hosts will immediately reply timeout. Defaults to true.

See #213 for more information

Daniel Mezzatto added 2 commits April 4, 2014 19:14
A boolean value that controls if auto ejected hosts should be dropped from the hash ring. If set to false, failing hosts will immediately reply timeout. Defaults to true.

See twitter#213 for more information
 - we dont need the timeout insertion code into the red-black tree since a dead server will not reach this code
 - forcing timeout error when a request is done against a dead server
@pankajsethi
Copy link

👍 will love to see it in master.

@mezzatto
Copy link
Contributor Author

mezzatto commented Jul 2, 2014

@manjuraj, when do you plan to accept this pull? Did you understood the feature? Please tell me if you need some more explanation.

@ghost
Copy link

ghost commented May 5, 2015

👍 definitely a good one! nice job.

@codekitchen
Copy link

Am I reading this change correctly that auto_eject_drop is only implemented in the modula distribution? That seems like a pretty big caveat.

@mezzatto
Copy link
Contributor Author

mezzatto commented Jun 9, 2015

@codekitchen, that's right.

In ketama distribution, this option doesn't seem to make sense (or does it?).

In random distribution, the implementation is indeed not done yet (but it's trivial to get done). I had no practical application for it when I first implemented the original code, so I though that the less code I touched the faster the pull would be accepted... :)

@codekitchen
Copy link

Heh well, I can't speak for the maintainers but I would certainly want the limitation to at least be documented.

I'm curious why you say it doesn't make sense for ketama -- to venture a guess, I think you're saying that why would you be using consistent hashing if you don't plan on modifying the ring on failure? In our case, it's because we want consistent hashing for when we explicitly resize the ring, but we also prefer this behavior of not modifying on node failure, rather just failing fast.

@mezzatto
Copy link
Contributor Author

@manjuraj, would you be willing to accept this pull If I documented that this feature is only for the modula distribution? Or do you think that, for completeness, it would be for the better to make the changes in ketama distribution as @codekitchen raised some valid use cases?

@mezzatto
Copy link
Contributor Author

mezzatto commented Apr 7, 2017

After all theses years, revisiting this PR made me realize that it's a https://en.wikipedia.org/wiki/Circuit_breaker_design_pattern

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Daniel Mezzatto seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mezzatto
Copy link
Contributor Author

@CLAassistant you are a bot robot. Of course I'm a github user :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants