CMFActivity.Activity.SQLBase: Do not increment priority after failure
When an activity failure happens, the SQL row is updated:
- date is set to a future value, so the activity does not get retried immediately, in the expectation that what caused the failure may have cleared by that point.
- retry is incremented, to allow limiting the total number of retries
- priority is incremented
This last point seems harder to justify, and seems redundant with the date increase. In the context of processing node families and with a steady influx of similar activities at a base priority level, such priority increment can postpone the victim activity execution to an arbitrarily large amount of time, which is undesirable.
So, remove this increment.
--
Some more notes on priorities: priorities can be (and in practice, are) used to allow batch processes to yield processing time to interactive uses. While specific priority values do not have any defined meaning, the increment mechanism being removed here means that in order to have activities depended upon by interactive use (ex: document indexation after a user created it) to execute before batch processes (ex: a long-running report), we would have to space the used priority values by the value of max_retry (ex: 1, 4, 7, 10, etc). I believe this is not usually done, and this causes crippling issues when the system is under unusually-high workloads, while low workloads hide this issue allowing it to proliferate silently.
This, plus my expectation that priority increment on failure was just never challenged in the last 15+ years, makes me think dropping it is a good way forward.
A possible side-effect I experienced is that tests which hid activity failures:
- before: first execution attempt fails, priority incremented, another activity is executed first thanks to the reordering, second attempt succeeds, test carries on
- after: first execution attempt fails, second execution attempt fails, tic raises, test fails
I believe such failure is simply the sign of a missing activity dependency, and adding such dependency is the correct fix - not incrementing priorities.
/cc @jm