feat(backends): add set_if_absent and delete_if_equals for locks and slots - #63
Merged
Merged
Conversation
…ng it `test_the_repository_changelog_can_be_released` runs the promotion script on the real CHANGELOG.md, but asserted that the previous release was v0.3.4 and that the notes open with `### Security`. Both were true of the 0.3.5 notes and stopped being true the moment 0.3.5 was released, so the test has been failing on master since. It now takes the previous version from the first released heading and only requires the notes to open with a section heading.
…slots Acquiring a lock or a per-user slot could only be emulated with `increment` plus a compensating decrement, and the two calls race: concurrent claimers can leave a slot held by nobody, and a release landing between someone else's increment and decrement frees a slot that is still in use. Releasing with a plain `delete` has its own race — a holder whose entry expired deletes the entry of whoever claimed the key after it. `set_if_absent(key, value, ttl=None) -> bool` claims a key only when it is free (an expired key counts as absent): Redis `SET NX EX`, Memcached `ADD`, memory under its lock. `delete_if_equals(key, expected) -> bool` releases only while the key still holds the caller's entry, compared as a decoded `CacheEntry` so counters match too. Redis compares in Python and deletes through a Lua script that re-checks the raw bytes it compared. Memcached's classic `DELETE` takes no CAS token, so it uses `GETS` and a `CAS` write with exptime -1, which the server treats as immediately expired and which a later `ADD` can claim. Both are non-abstract with non-atomic fallbacks on `BaseCacheBackend`, so third-party backends keep working. The race tests for Redis and Memcached overwrite the key between compare and delete; swapping either implementation for a plain delete fails exactly those two tests. Closes #62
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 #62
What
Two atomic primitives on
BaseCacheBackend, overridden by every built-in backend:set_if_absent(key, value, ttl=None) -> booldelete_if_equals(key, expected) -> boolSET NX EXADDGETS+CASwrite with exptime-1(immediately expired)Why
delete_if_equalstoo#62 proposes releasing with
delete. That frees the wrong holder when the first holder's entry has expired and someone else has claimed the key since — likely with long-lived SSE streams. Releasing only while the key still holds your token closes that.Notes
DELETEtakes no CAS token, so the release is a CAS write with a negative exptime. Verified against a real server: a stale CAS token fails, and a subsequentADDclaims the key.CacheEntry, so counters (stored as bare integers on Redis/Memcached) compare correctly.BackendProxyneeds no change: callers already reach the backend viaBackendProxy.get().test_the_repository_changelog_can_be_releasedhardcodedv0.3.4and### Security, so it has been failing on master since 0.3.5 was released. It now reads the latest release from the file.Testing
deletefails exactly those two tests.mypy --strictand pre-commit clean.