Restarting the engine unconditionally resets the max_concurrency values
This one is a bit complicated, but:
Preamble
A test case script:
import time
import spinach
sp = spinach.Engine(spinach.RedisBroker())
@sp.task(name='nap', max_retries=1, max_concurrency=8)
def nap(job_id):
print(f"{job_id:3} Zzzz...")
time.sleep(5)
batch = spinach.Batch()
for i in range(32):
batch.schedule('nap', i)
sp.schedule_batch(batch)
sp.start_workers(32, stop_when_queue_empty=True)
Steps to reproduce
docker-compose -f spinach/tests/docker-compose.yml up -d- Run the script above, it should process the queue 8-at-a-time
- Run
redis-cliand execute the commandHSET spinach/_max_concurrency nap 32 - Run the script again
Expected result
The second run of the script runs all 32 tasks simultaneously, like we told it to
Actual behavior
The second run of the script processes the queue 8-at-a-time
Miscellany
- This is not a bug; it is 100% intentional behavior, with comments in the
set_concurrency_keys.luascript explaining that it is done on purpose. - Our app has several Spinach tasks with
max_concurrencyset to "baseline" values, and Operations occasionally has a need to tune the values up or down, for reasons. - This is highly inconvenient for things like
flask_spinachrunning under Gunicorn; when Gunicorn cycles out workers, the new workers start and destroy any runtime adjustments to the Redis that operators may have made; and it does so with no warning. - Conversely, it is highly convenient if an Operator wants to, e.g., set a "burst" value for a Spinach task, and immediately forget about it; the software will put itself back on its own.
- The previous behavior can be simulated by putting a TTL on the
_max_concurrencykey, but Issue #15 makes it not work very well. - Swinging the concurrency adjustments through the app, rather than fidgeting with Redis underneath it, is also an obvious dodge to this issue.
Any thoughts and insights on how to best address this would be appreciated.
This is not a bug; it is 100% intentional behavior, with comments in the set_concurrency_keys.lua script explaining that it is done on purpose.
This is the only place that sets up the key, so naturally it's going to override the current value if one was set from a different client.
I'd advocate for a second key that allows user to set their own limits, effectively as an override for the code's own limit. We can amend the Lua script so it looks for that key first and falls back to the real one. Users could potentially add a TTL on the second key to get the "temporary behaviour" action.
In fact I'd say let's add a tool script to do this, so that the user doesn't need to know about our Redis keys. Something like:
spinach max_concurrency myjobname 32 1m
to set myjobname job to 32 concurrency for 1 minute.