Conversation
thomasst
left a comment
There was a problem hiding this comment.
- How would you handle
update_scheduled_time? - Have you considered just using
task.tsfor this?
Will add handling for it, thanks! If
Yes, that was my initial approach and it's what is used right now for the metric in https://github.com/closeio/closeio/pull/50195 but it's incorrect for this purpose - |
tasktiger/task.py
Outdated
| ) | ||
|
|
||
| self._data["scheduled_at"] = ts | ||
| tiger.connection.set(tiger._key("task", self.id), json.dumps(self._data)) |
There was a problem hiding this comment.
Should this be part of the pipeline?
The other thing to consider is that if the _data is stale for some reason, it will just overwrite the existing data. I'm not sure if there is a specific case where this would turn into an issue.
There was a problem hiding this comment.
I thought about it but considered against it - if the Task is not in the queue/SCHEDULED state, we'll still overwrite the data without actually modifying the scheduled time in the queue.
Maybe I can write a Lua script to do all this atomically - I'll give it a go.
|
Achieved atomicity with Lua - including patching the |
| UPDATE_SCHEDULED_TIME = """ | ||
| local score = redis.call('zscore', KEYS[1], ARGV[2]) | ||
| if score then | ||
| redis.call('zadd', KEYS[1], ARGV[1], ARGV[2]) |
There was a problem hiding this comment.
zadd mode=XX is not necessary in this script since we check for score upfront
We want to track E2E delay from when a task was scheduled to when it finally ended up being executed. We cannot use the
Task.tsbecause that gets reset tonow()every time the task changes state.