CI job running tests with queuing on

[gjoseph92]

Adds a CI job, only for Ubuntu and py3.9, running the test suite with worker-saturation: 1.0 (instead of inf).

Also adds a @pytest.mark.oversaturate_only marker, which automatically skips marked tests if queuing is enabled.

A surprisingly small number of tests actually failed when queuing was turned on. I've updated the ones that did as needed. I've tried as much as possible to just make the tests agnostic to queuing. In a few cases though, I've needed to add explicit conditional logic. And in a couple cases, I've marked tests to be skipped when queuing is enabled, since they may not make sense.

Unfortunately there are three tests that are flaky under queuing, which could indicate an actual bug. They're skipped for now, but should be investigated.

Closes #6631

• Tests added / passed
• Passes pre-commit run --all-files

[github-actions]

Unit Test Results

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

       15 files  ±0         15 suites  ±0   5h 53m 29s ⏱️ - 16m 4s
  3 082 tests ±0    2 996 ✔️  -   1    85 💤 +  1  1 ❌ ±0 
22 808 runs  +6  21 895 ✔️ +79  912 💤  - 73  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 33e4995. ± Comparison against base commit 1818788.

♻️ This comment has been updated with latest results.

[crusaderky]

Instead of this new oversaturate_only mark, couldn't you simply have

OVERSATURATION = math.isinf(dask.config.get("distributed.scheduler.worker-saturation"))
@pytest.mark.flaky(not OVERSATURATION, reason="flaky on MacOS")

Tests that don't make sense with queueing should instead force

config={"distributed.scheduler.worker-saturation": math.inf}

(some already do)

[fjetter]

I think this renaming will break some logic in our test report generation.

distributed/continuous_integration/scripts/test_report.py

Lines 146 to 152 in acf6078

	df_jobs["suite_name"] = (
	df_jobs["OS"]
	+ "-"
	+ df_jobs["python_version"]
	+ "-"
	+ df_jobs["partition"].str.replace(" ", "")
	)

distributed/continuous_integration/scripts/test_report.py

Lines 353 to 357 in acf6078

	html_url = jobs_df[jobs_df["suite_name"] == a["name"]].html_url.unique()
	assert (
	len(html_url) == 1
	), f"Artifact suit name {a['name']} did not match any jobs dataframe {jobs_df['suite_name'].unique()}"
	html_url = html_url[0]

I introduced this in #6837

Worst case, we remove the more specific job URL again (we knew from the start this is very brittle)

[fjetter]

see https://github.com/dask/distributed/pull/6837/files#r939182529

[fjetter]

I don't think this should block this PR

[gjoseph92]

• Removed the oversaturate_only mark; just using pytet.mark.skipif and config={"distributed.scheduler.worker-saturation": math.inf} as necessary

• Fixed setting the environment variable in the test step

• Updated test report script to handle and display results from the queue job:

  

Ready for re-review and merge.

[gjoseph92]

• Flaky test_steal_reschedule_reset_in_flight_occupancy #6999

[crusaderky]

Thank you!

[jrbourbeau]

Just to confirm, is the new CI job added here for testing purposes and is planned to be removed in the future, or is this intended as a long-term change?

[crusaderky]

Long term, we'll want to enable scheduler-side queueing by default.
When that happens, we'll need to revisit the unit tests to have selected few tests with queueing off (like it already happens with all other config toggles) and revert this PR.
Before that, all tests that are flaky only when queueing is on need to be addressed.