_transition_from_resumed contains legacy code and documentation

[hendrikmakait]

_transition_from_resumed should not be reachable from a code path where ts._previous == "executing". Yet, it states this in the docstring and performs checks for this behavior. We should revisit the code and explanation and clean it up to improve readability.

[crusaderky]

These three are really the same issue:

• Resumed tasks don't release resources if they fail #6682
• Deadlock when a flaky task is stolen #6689
• _transition_from_resumed contains legacy code and documentation #6693

distributed/distributed/worker_state_machine.py

Lines 1948 to 1968 in 40867c7

	if ts._previous == finish:
	# We're back where we started. We should forget about the entire
	# cancellation attempt
	ts.state = finish
	ts._next = None
	ts._previous = None
	elif not ts.done:
	# If we're not done, yet, just remember where we want to be next
	ts._next = finish
	else:
	# Flight/executing finished unsuccessfully, i.e. not in memory
	assert finish != "memory"
	next_state = ts._next
	assert next_state in {"waiting", "fetch"}, next_state
	assert ts._previous in {"executing", "long-running", "flight"}, ts._previous
	if next_state != finish:
	recs, instructions = self._transition_generic_released(
	ts, stimulus_id=stimulus_id
	)
	recs[ts] = next_state

Lines 1949-1953 are unreachable, because if ts._previous == finish: does not do what the author thought:

• ts._previous is either executing, long-running, or flight
• finish is either waiting, or fetch, or missing

As a side note, I'm unsure how a transition resumed->missing can ever happen, but I'm fairly sure there's no unit test for it.

As a consequence, the use case

• flight->cancelled->resumed(waiting)->fetch
• executing->cancelled->resumed(fetch)->executing
• long-running->cancelled->resumed(fetch)->long-running

is unwittingly covered by lines 1958+.

Those lines do not cover the use case # Flight/executing finished unsuccessfully, i.e. not in memory.

• If executing fails, the transition resumed->error is performed by _transition_generic_error, which causes all sorts of problems (Deadlock when a flaky task is stolen #6689).
• if flight fails with GatherDepFailureEvent, you also have the same _transition_generic_error which... might? ... work as intended (missing a test).
• if flight fails with GatherDepNetworkFailureEvent or GatherDepSuccessEvent (but no data), then you do have a transition to executing served by lines 1958+, and that's tested by test_workerstate_flight_failure_to_executing.