fix(memcached): raise while a server is unreachable instead of returning defaults - #216
Merged
Merged
Conversation
…ing defaults With pymemcache's default retries, HashClient answered every call to a server that had just failed with the command's default until the retry was due: get() looked like a miss, set() dropped the write, increment() returned 0 and delete_if_equals() raised TypeError on the missing CAS pair. The client now uses retry_attempts=0, so a failed server leaves rotation at once and calls raise, and dead_timeout=1, so it is tried again after one second rather than 60. Closes #197
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #197
Problem
MemcachedBackendbuilt itsHashClientwith pymemcache's default retry settings (retry_attempts=2,retry_timeout=1,dead_timeout=60). After a call to a server failed, every call to that server returned the command's default value without raising, until the retry was due. On a single-server backend, one failedget()made the next calls return:get/get_and_deleteNone(looks like a miss / already consumed)setset_if_absentFalse(looks like the key is taken)increment0(a rate limiter sees a fresh counter)delete_if_equals/expire_if_equalsTypeErrorfrom unpackingNonedelete_many(["a", "b"])2delete/clear_pathNone/0After two more failures the server was marked dead for 60 seconds.
Change
The client is now built with
retry_attempts=0anddead_timeout=1. Withignore_exc=False(unchanged), this leaves no path inHashClientthat returns a default:MemcacheError("All servers seem to be down right now")for a single server.With several servers, a failed server's keys go to the remaining servers until it answers again. That is
HashClient's normal failover; before this change it started after the third failure instead of the first.Docs (en and zh-TW
BACKENDS.md) describe the behaviour, and the CHANGELOG has a Fixed entry.Verification
tests/backends/test_memcached.py. They need no server:test_memcached_keeps_raising_after_a_connection_failure, parametrized over the ten methods;test_memcached_tries_a_failed_server_again_after_the_dead_timeout.retry_attempts=0fails the ten parametrized cases and nothing else;dead_timeoutfails only the dead-timeout test.mypy --strict, the full suite against live Redis and Memcached (893 passed), andzensical build --strictfor both languages.Note
This does not touch
get_and_delete's logic, so it does not conflict with the CAS change planned in #175.