A reaper retry decision can overwrite a task that someone else already handled
Les mainteneurs répondent en général sous 1 jour
Personne n'a encore pris cette issue.
Évaluation
- Difficulté
- 4/5
- Temps estimé
- 3-5 jours
- Accessibilité débutants
- 64/100
Piste de recherche
Read reaper.lua, threadmill/backends/lua/acknowledge.lua, and the Redis backend in threadmill/backends/redis.py, then inspect commit 382880b and the TestRedisBrokerReap guard tests. Trace how the claim reaches acknowledge and requeue decisions. Done means stale decisions are dropped while matching-claim acknowledge and requeue paths pass, including the worker-ack, claim-takeover, and inspector-action race tests.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
After the lease-expiry retry change (commit 382880b), the reaper hands expired tasks to the retry callback:
reaper.luaclaims expired running entries. It renews their lease toRedisBroker.CLAIM_TTLand returns the IDs. It keeps the task data hash.RedisBroker._reap_taskdeserializes each claimed task, appends theAcknowledgementTimeouterror, and evaluates theretrycallback of the task. Then it callsbackend.requeue(...)to retry orbackend.acknowledge(...)to finalize.
The claim protects against two brokers that claim the same task in the same pass. The select-and-renew step in the script is atomic. But the decision does not make sure that the broker still holds the claim. A stale decision can act on a task that someone else already handled:
- Late worker acknowledgement (single node). A slow task finishes after its lease expires. The
acknowledge()call of the worker storesSUCCESSFULand removes the task hash. If the broker read the data before that, itsrequeue()removes the result and overwrites the task data. It also adds the task to the deferred set again. The task runs again although it succeeded. - Stalled broker, claim taken over. Broker A claims a task and then stalls past
CLAIM_TTL(a GC pause, a slow retry callback, or a network problem). Broker B claims the task again and completes the decision. When A starts again, its stale decision overwrites the outcome from B and can schedule the task twice. - Inspector action. A user requeues or removes the task between the claim and the decision. The decision of the broker undoes that action.
The finalize path is mostly protected by the ZREM guard in acknowledge.lua. A second acknowledgement is a no operation. The dangerous operation is mainly requeue, which returns the task to the queue. All paths can also overwrite the task data.
Proposed correction
Make the reap decision conditional on the claim that produced it:
- Let
reaper.luawrite a claim identity with the running entry. Use the claim deadline and compareZSCORE, or use a token in the task hash. - Give
acknowledge()andrequeue()an optional guard parameter. The Lua scripts must make sure that the parameter matches before they write. A mismatch discards the decision. The claim then lapses and the next pass decides again. The result is a delay, not a lost task. - Tests: the worker-ack race, claim takeover after
CLAIM_TTL, inspector dequeue between claim and decision, and the matching-claim path for bothacknowledgeandrequeue.
An implementation of this guard was written and then removed to keep the lease-expiry retry change small. The code can return from commit 382880b (files threadmill/backends/lua/acknowledge.lua, threadmill/backends/redis.py, and the TestRedisBrokerReap guard tests).
- Langage dominant
- Python
- Étoiles
- 19
- Forks
- 1
- Merge moyen
- 14 h 39 min
- PR mergées (30 j)
- 21
Préparer son environnement
- Aucun Dockerfile ni fichier Docker Compose
- Aucun modèle de pull request
- Lire le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de codingjoe/threadmill
-
Add worker pool telemetryPeut-être à nouveau libre @codingjoe l’a pris il y a 99 jours, et aucune pull request n’est ouverte. Ouverteenhancement
codingjoe/threadmill#18 · 1 personne assignée ·
Les mainteneurs répondent en général sous 1 jour
Toutes les issues de codingjoe/threadmill
Issues similaires
-
Difficulté 2/5 1-3 heures Accessibilité débutants 82/100
RedHatQE/mtv-api-tests#721 ·
Les mainteneurs répondent en général sous 1 jour
-
Difficulté 2/5 1-3 heures Accessibilité débutants 84/100
Les mainteneurs répondent en général sous 1 jour
-
Difficulté 1/5 1-3 heures Accessibilité débutants 85/100
pytest-dev/pluggy#757 ·
Les mainteneurs répondent en général sous 1 jour
-
Difficulté 1/5 1-3 heures Accessibilité débutants 85/100
NousResearch/hermes-agent#134960 ·
Les mainteneurs répondent en général sous 1 jour
-
HTML backend: `<br>` leaks the internal sentinel U+E000 into list items, headings and captionsPeut-être pris @morten-lagabote l’a pris aujourd’hui. Ouverte
Difficulté 2/5 1-3 heures Accessibilité débutants 67/100
docling-project/docling#4671 ·
Les mainteneurs répondent en général sous 1 jour