Better logging for worker removal

[crusaderky]

Improve all regular logger output as well as Scheduler.events around worker shutdown.

• print a WARNING when tasks are going to be recomputed due to worker failure
• print an ERROR when scattered data is lost due to worker failure
• Graceful worker retirement will now warn when it fails to retire a worker (as no other worker can accept its unique data)
• Scheduler.events now gives a more complete picture of graceful worker retirement as well as worker failure

@jrbourbeau best effort for 2024.2.1

[github-actions]

Unit Test Results

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

    27 files  +    4      27 suites  +4   9h 58m 0s ⏱️ + 2h 18m 2s
 3 997 tests +    1   3 885 ✅ +    1    110 💤 ±  0  2 ❌ ±0 
50 267 runs  +9 037  47 967 ✅ +8 647  2 295 💤 +389  5 ❌ +1 

For more details on these failures, see this check.

Results for commit 7db2c8b. ± Comparison against base commit cbf939c.

This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.

distributed.tests.test_scheduler ‑ test_gather_failing_cnn_recover

distributed.tests.test_scheduler ‑ test_gather_failing_can_recover
distributed.tests.test_worker ‑ test_log_remove_worker

♻️ This comment has been updated with latest results.

[crusaderky]

This whole block is redundant with the same inside Client._close.
This changes the close() and __exit__() behaviour of a synchronous Client that started its own LocalCluster (e.g. client = Client()), so that the client first closes itself, thus releasing all tasks on the scheduler, and only afterwards it closes the cluster instead of the other way around.

Note that asynchronous clients already behave this way.

Without this, as the workers close, the scheduler would observe the sudden loss of all tasks the client had in who_wants.
See test: test_quiet_client_close

[crusaderky]

Could have used True/False but this is IMHO more readable

[hendrikmakait]

Thanks, @crusaderky! Consider the nits non-blocking.

[hendrikmakait]

Should we also log the erred tasks here?

[crusaderky]

You don't lose the exception of erred tasks when you lose the workers where they ran.

[hendrikmakait]

I know that this is only cosmetical but is there a better stop condition here? E.g., n tasks being in memory already? Consider this non-blocking.

[crusaderky]

Done

[hendrikmakait]

It looks like the rather verbose test_log_remove_worker fails consistently.