Fix crash in spans when time() is not monotonic

[crusaderky]

Fix crash:

  File "/opt/coiled/env/lib/python3.11/site-packages/distributed/spans.py", line 328, in cumulative_worker_metrics
    unknown_seconds = max(0.0, self.active_cpu_seconds - known_seconds)
                               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/coiled/env/lib/python3.11/site-packages/distributed/spans.py", line 439, in active_cpu_seconds
    return sum((t1 - t0) * nthreads for t0, t1, nthreads in self.nthreads_intervals)
                                                            ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/coiled/env/lib/python3.11/site-packages/distributed/spans.py", line 410, in nthreads_intervals
    is_active_t, is_active_flag = zip(*self._active_timeseries())
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/coiled/env/lib/python3.11/site-packages/distributed/spans.py", line 382, in _active_timeseries
    assert delta > 0
           ^^^^^^^^^
AssertionError

This was triggered when two subsequent calls to time() are not strictly monotonic ascending.
No unit tests, as I'm not too eager to monkey-patch time() to reproduce

[ntabris]

Mostly just curious but if the problem is that time() isn't monotonic, why isn't the solution to use a monotonic clock?

[crusaderky]

Mostly just curious but if the problem is that time() isn't monotonic, why isn't the solution to use a monotonic clock?

Because time.monotonic() is not a wall clock, but just the number of seconds since process start.
In Windows we tamper with time.time() due to low resolution. In theory, we could do the same in Linux to create a monotonic wall time. In practice, it would be ugly as hell (see distributed.metrics._WindowsTime), slow, and very bug-prone.
So I feel it's much better to just work within the constraints of the standard UNIX syscalls.

[github-actions]

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       20 files  ±       0         20 suites  ±0   11h 29m 50s ⏱️ + 38m 22s
  3 704 tests ±       0    3 596 ✔️ +    4     106 💤 ±  0  2 ❌  - 4 
35 826 runs  +1 019  34 076 ✔️ +971  1 748 💤 +52  2 ❌  - 4 

For more details on these failures, see this check.

Results for commit 0d8ee46. ± Comparison against base commit a47cb0a.

♻️ This comment has been updated with latest results.

[hendrikmakait]

IIUC, child.enqueued <= child.stop. How about sorting by timestamp only events.sort(key=lambda event: event[0])? This should maintain relative ordering and avoid n_active < 0. (It took me a moment to understand why n_active would ever drop below 0 here.)

[crusaderky]

Done

[hendrikmakait]

LGTM!