A late acknowledgement can overwrite the retry attempt that replaced it

[codingjoe]

Found while reviewing PR #69 with the SuperJoe crew. The error predates that patch and the patch does not touch it. Filed as out-of-scope there.

acknowledge.lua fences only on the membership in the running set. It ZREMs the id and continues only when the id was there. A worker whose lease expired but that still runs can acknowledge after the reaper requeued the task for a retry. The claim of the reaper re-armed the running entry, so the ZREM of the late ack succeeds. It then writes the stale result, removes the task hash, and removes the running entry of the new attempt. The deferred entry that the retry created survives and runs the task again.

The README already documents that a retry can run at the same time as a task that outlived its lease. Duplicate execution is therefore expected. The late ack publishing a result for the wrong attempt and overwriting the state of the retry is not expected.

Options:

• Fence the ack on the attempt it was leased for. A per-attempt token beside the lease, compared by the script, is one option.
• Or let an ack lose to a renewed claim.

Reference: threadmill/backends/lua/acknowledge.lua.

[codingjoe]

Closing as a duplicate of #75. Same finding: acknowledge.lua fences only on running-set membership, so a late acknowledgement from an expired attempt can clobber the retry attempt that replaced it. #75 is older and already names the fix direction, fencing the ack on the attempt it was leased for.