Review log-length configuration #8173
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 21 files ± 0 21 suites ±0 10h 56m 21s ⏱️ - 3m 13s Results for commit 7e780c7. ± Comparison against base commit b6333df. ♻️ This comment has been updated with latest results. |
crusaderky
force-pushed
the
log-length
branch
5 times, most recently
from
73879f5
to
2d96551
Compare
crusaderky
force-pushed
the
log-length
branch
from
2d96551
to
b967e78
Compare
distributed/utils.py
Outdated
| "deserialize_for_cli": "dask.config.deserialize", | ||
| "serialize_for_cli": "dask.config.serialize", | ||
| "format_bytes": "dask.utils.format_bytes", | ||
| "format_time": "dask.utils.format_time", | ||
| "funcname": "dask.utils.funcname", | ||
| "parse_bytes": "dask.utils.parse_bytes", | ||
| "parse_timedelta": "dask.utils.parse_timedelta", | ||
| "typename": "dask.utils.typename", | ||
| "tmpfile": "dask.utils.tmpfile", |
There was a problem hiding this comment.
I'm curious why these are removed?
There was a problem hiding this comment.
This is part of #8171
crusaderky
force-pushed
the
log-length
branch
from
1bec147
to
edbea05
Compare
crusaderky
force-pushed
the
log-length
branch
3 times, most recently
from
8a5a9d3
to
b5f1a1f
Compare
distributed/system_monitor.py
Outdated
| if maxlen is no_default: | ||
| maxlen = dask.config.get("distributed.admin.log-length") | ||
| if isinstance(maxlen, int): | ||
| maxlen = max(1, maxlen) |
There was a problem hiding this comment.
I'm not sure how I feel about lumping this in with the other logs.
Barely anybody will care for the scheduler/worker transition log
However, this will cut the system dashboard page, won't it?
There was a problem hiding this comment.
Moved to distributed.admin.system-monitor.log-length
distributed/config.py
Outdated
| "ucx": "distributed.comm.ucx", | ||
| "rmm": "distributed.rmm", | ||
| # log-length aliases | ||
| "transition-log-length": "distributed.admin.log-length", |
There was a problem hiding this comment.
distributed.admin.log-length is the length of the actual logs we keep. There is even an option admin.log-format.
Some users care about this (it enables client.get_logs() even if it is just partial logging).
The logs we intend to cut are low level debug logs and I think we should distinguish this
There was a problem hiding this comment.
Moved everything else to distributed.admin.low-level-log-length
crusaderky
force-pushed
the
log-length
branch
3 times, most recently
from
8885393
to
24eb71d
Compare
crusaderky
force-pushed
the
log-length
branch
from
24eb71d
to
83e077b
Compare
n_workersdeprecation inwait_for_workers#8192distributed.utils#8193distributed/config.pybut otherwise don't block merging:dask.configdask#10499dask.config.set()#8179Make all deque lengths configurable from a single config line, and set the default to 1000.
[EDIT]
Previously this PR was setting the default to 0. I think it was problematic as it would cause
get_scheduler_logs()andget_worker_logs()to return no results. In a separate PR I will print a warning on the client when maxlen is reached.Reproducer from #8164 (the difference in duration looks like noise):
