Scheduler transitions should not swallow kwargs #5926
Conversation
Unit Test Results 8 files - 4 8 suites - 4 3h 1m 17s ⏱️ - 2h 41m 53s For more details on these failures, see this check. Results for commit cfcf7d6. ± Comparison against base commit f9d2f91. ♻️ This comment has been updated with latest results. |
|
Blocked by #5932. As long as it's there I can't tell if I'm introducing regressions or not. |
fjetter
left a comment
fjetter
left a comment
There was a problem hiding this comment.
I think the tests
FAILED distributed/dashboard/tests/test_scheduler_bokeh.py::test_task_stream
FAILED distributed/dashboard/tests/test_scheduler_bokeh.py::test_task_stream_n_rectangles
FAILED distributed/dashboard/tests/test_scheduler_bokeh.py::test_task_stream_clear_interval
FAILED distributed/dashboard/tests/test_scheduler_bokeh.py::test_lots_of_tasks
FAILED distributed/diagnostics/tests/test_eventstream.py::test_eventstream - ...
FAILED distributed/diagnostics/tests/test_task_stream.py::test_TaskStreamPlugin
FAILED distributed/diagnostics/tests/test_task_stream.py::test_get_task_stream_saveare failing because we remove this information. If we want to clean this up further, we should probably submit a metadata kwarg in the task-erred/finished messages to make this explicit instead of implicit
| if ts.startstops: | ||
| smsg["startstops"] = ts.startstops |
There was a problem hiding this comment.
I think this is still used for our task stream. Erred tasks should be visualized as black bars. If this is not the case, this might be a regression
| "op": "task-erred", | ||
| "status": "error", | ||
| "key": ts.key, | ||
| "thread": self.threads.get(ts.key), |
There was a problem hiding this comment.
I believe this is used in the dashboard to assign the startstops to the proper lane
| "status": "OK", | ||
| "key": ts.key, | ||
| "nbytes": ts.nbytes, | ||
| "thread": self.threads.get(ts.key), |
There was a problem hiding this comment.
same here, I believe this is required
|
I think this seemingly innocent cleanup - which turns out to break everything everywhere - highlights a fundamendal design problem, which is that Worker is sending data to Scheduler but there is no straighforward way to figure out where, if anywhere, it is used - short of trying to blindly remove it and see if any tests break. Anyway this is on hold for now. I'll see at a later date if there is anything I can salvage out of this. |
|
This is ancient and the effort required in trying to salvage any of it would not justify the returns. Closing. |
2 participants
Broken out of #5922
{"op": "task-finished"}worker message was sending unused keys "status" and "thread", which were silently swallowed by the**kwargsoftransition_processing_memoryandtransition_waiting_memory.{"op": "task-erred"}worker message was sending unused keys "status", "thread", and "startstops", which were silently swallowed by the**kwargsoftransition_processing_erred.