[Swarming] Reconsiders swarming tasks for remote execution

[IvanBM18]

We laid the ground for swarming scheduling quite some time ago, but due to other blockers(i.e. android fuzzing issues), we never really tested the scheduling in prod.

The issue here is that batch_service.is_remote_task() only returns true for tasks whose platform has a matching config in the batch.yaml file, i suspect we didn't saw this before because until recently, android emulator fuzzing, was scheduled as a LINUX platform, which is considered in batch config, were as ANDROID_EMULATOR is not

Changes

• When finishing a utask's preprocess step we know check for both swarming & regular batch tasks
• Added Unit Tests to check that a task's execution is redirected or either executed locally based on the environment, the job & the current task

Tests

• In prod all of the assertion errors i could find in the last week were related to the assertion error thrown when a swarming job is not considered in the batch assertion, i queried to find if there were other cases of this happening and couldn't find any:

Swarming job Before:

After: With this small change swarming task are now successfully redirected to the utask_main queue:

So that they are processed by the utask_main scheduler

[dylanjew]

mostly LGTM. just a couple questions

[dylanjew]

LGTM!

[PauloVLB]

Just a heads up on something this unblocks.

When swarming can't take a task (queue full, API error), the gate hands it over to gcp_batch:

clusterfuzz/src/clusterfuzz/_internal/remote_task/remote_task_gate.py

Lines 192 to 195 in d6d9c25

	unscheduled_tasks.extend(service.create_utask_main_jobs(tasks))
	except Exception: # pylint: disable=broad-except
	logs.error(f'Failed to send {len(tasks)} tasks to {adapter_id}.')
	unscheduled_tasks.extend(tasks)

And batch resolves the specs for the whole list before creating anything, so one task whose platform isn't in the batch config kills the entire chunk:

clusterfuzz/src/clusterfuzz/_internal/batch/service.py

Line 493 in d6d9c25

specs = _get_specs_from_config(remote_tasks)

Saw it in dev with ValueError: No mapping for ANDROID_EMULATOR-PREEMPTIBLE-UNPRIVILEGED:

It doesn't hurt much in dev because gcp_batch_jobs_frequency is 0.2, so the slice is a prefix of the list and swarming tasks are appended last, meaning most cycles only send regular tasks to batch. In prod the frequency is 1.0, so the slice is always the whole list: one bad swarming task fails the whole cycle, and since nothing gets acked it comes back a few minutes later and fails again.

Not asking to change this PR, but I think we should make batch skip the tasks it can't map instead of raising for the whole list before we let this run in prod.

[dylanjew]

Just a heads up on something this unblocks.

When swarming can't take a task (queue full, API error), the gate hands it over to gcp_batch:

clusterfuzz/src/clusterfuzz/_internal/remote_task/remote_task_gate.py

Lines 192 to 195 in d6d9c25

	unscheduled_tasks.extend(service.create_utask_main_jobs(tasks))
	except Exception: # pylint: disable=broad-except
	logs.error(f'Failed to send {len(tasks)} tasks to {adapter_id}.')
	unscheduled_tasks.extend(tasks)

And batch resolves the specs for the whole list before creating anything, so one task whose platform isn't in the batch config kills the entire chunk:

clusterfuzz/src/clusterfuzz/_internal/batch/service.py

Line 493 in d6d9c25

specs = _get_specs_from_config(remote_tasks)

Saw it in dev with ValueError: No mapping for ANDROID_EMULATOR-PREEMPTIBLE-UNPRIVILEGED:

It doesn't hurt much in dev because `gcp_batch_jobs_frequency` is 0.2, so the slice is a prefix of the list and swarming tasks are appended last, meaning most cycles only send regular tasks to batch. In prod the frequency is 1.0, so the slice is always the whole list: one bad swarming task fails the whole cycle, and since nothing gets acked it comes back a few minutes later and fails again.

Not asking to change this PR, but I think we should make batch skip the tasks it can't map instead of raising for the whole list before we let this run in prod.

This sounds like something we should fix, but are you thinking it's not a blocker because the frequency of swarming not being able to take the task and falling back to gcp_batch is relatively low? If it continously fails and doesn't ack that sounds concerning

We can discuss in a separate bug/PR, but I also would expect failed swarming tasks to get retried on swarming, and failed batch tasks to get retried on batch, rather than skipping any retries for swarming tasks that we can't map to batch.

[PauloVLB]

but are you thinking it's not a blocker because the frequency of swarming not being able to take the task and falling back to gcp_batch is relatively low?

I assumed that when looking into the queue full problem, that's why I didn't treat this as a blocker for the PR. But there are other reasons for which swarming may refuse the task, like the job being deleted or the config changing after the task was queued, and those go to batch. And they don't recover on their own, dev has had the same 28 tasks in utask_main-swarming for 2+ days hitting the fallback:

And agree on the retries. I think the root of it is that _handle_swarming_jobs returns a single list that mixes "not a swarming task" with "swarming task I couldn't schedule", so the gate can't tell them apart and sends both to batch. If we split those, the declined swarming tasks just get nacked and retried on swarming on the next cycle, no cross backend at all. Which is better anyway, since something like ANDROID_EMULATOR can't run on batch even if we wanted to.

I'd still make batch skip the tasks it can't map, but that's a separate bug: any deleted job already does the same thing today, no swarming involved.

[IvanBM18]

Thanks for your findings paulo, Will leave this PR as it is (approved but not merged) and will stack a new one top of it changes so that unscheduled swarming tasks doesn't overthrows the whole batch of tasks as invalid.

Regarding the 28 tasks, i am positive said tasks are because the job no longer exists in the db, because currently the job-exporter is broken and besides breaking issues(like missing spec or config) would show more errors.