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

Surface 'server_disconnected' state to API consumers #238

Open
tonycosentini opened this issue Oct 28, 2020 · 2 comments
Open

Surface 'server_disconnected' state to API consumers #238

tonycosentini opened this issue Oct 28, 2020 · 2 comments

Comments

@tonycosentini
Copy link

Would it be possible to surface the cases where there are connectivity errors?

For example with get right now, it returns None when there is a connectivity error and there is no way to distinguish if the key is actually missing or if there was a connectivity error.

I'd be happy to open a PR that adds this functionality via raising some kind of exceptions. I think to keep API compatibility enabling this would need to be an option when creating a client.

@jaysonsantos
Copy link
Owner

jaysonsantos commented Nov 25, 2020

Hey there, I try to mimic what happens on pylibmc for example. Do you know what it returns?
I guess that if you want this behavior, you could extend Protocol and on _connection_error you could raise the exception.
Right now, when the socket is disconnected, disconnect is called so on the next get it should try and reconnect and as this is caching the data is dealt with in a way that it can be recomputed when cache fails, I think that it is better than raising an exception and breaking the client's app flow.
If you want to distribute the load and increase availability, I would suggest using the DistributedClient because it will spread the load based on the key so if a caching server fails, it should affect only a portion of your load

@tonycosentini
Copy link
Author

Hey @jaysonsantos, thanks for the guidance here!

I think pymc is raising ValueErrors - I'm not super familiar with the library, but looking at the C source, it looks like it calls PyErr_SetString in a bunch of cases.

For some additional context - we are using the distributed client. Our use case is using this library + flask-limiter. With flask-limiter, there are multiple get memcache calls that need to succeed (for example, get a rate limit expiration). If one of them errors, it's possible to end up extending rate limiting windows. This is rare, but can happen often depending on the request throughput you are doing. (Sorry if this is super hand-wavey).

Anyway, I'll try extending the protocol like you said. If it works out for our flask-limiter case I can open a PR to try to introduce the behavior without breaking the API (maybe make it opt-in or something).

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

No branches or pull requests

2 participants