Fix infinite loop in MPITaskScheduler

[yadudoc]

Description

Currently, the MPITaskScheduler's schedule_backlog_tasks method takes tasks from the backlog and attempts to schedule them until the queue is empty. However, since calling put_task pops the task back onto the backlog queue, this ends up in an infinite loop if at least 1 task cannot be scheduled. This PR fixes this bug and a related performance issue with tasks potentially getting unpacked multiple times.

Changed Behaviour

• This PR fixes the bug by updating schedule_backlog_tasks to fetch all tasks in the backlog_queue and then attempt to schedule them avoiding the infinite loop.
• put_task is divided into packing and scheduling tasks with a new schedule_task method. Backlog processing calls schedule_task to avoid redundant unpacking.

These are change split from #3783 to keep the PR concise.
This is split 2 of 3.

Type of change

• Bug fix
• Code maintenance/cleanup

[benclifford]

pending_task_q is a multiprocessing.Queue and so some (implicit) pickle serialization happens there (I'm not sure exactly when) in order to get the objects movable between processes. So that makes me immediately suspicious about "a related performance issue with tasks potentially getting unpacked multiple times." without seeing some numbers: although the Parsl code is not reserializing everything, is Python reserializing and deserializing everything each time round when it puts things on the multiprocessing queue?

[benclifford]

i guess that doesn't matter too much - a task only goes into the multiprocessing queue once, and lives in the queue.PriorityQueue backlog queue the result of the time which doesn't do serialization.

[yadudoc]

Alright, here's the difference: MPITaskScheduler must unpack, get node info and repack the task with it and put the task into the pending_task_q. These steps are required. However, we were unpacking all backlogged tasks whenever backlogged tasks were called, which is unnecessary and could be costly. I've not measured perf improvements here, but it doesn't seem like a wasted effort.

[benclifford]

i think its fine - i was misunderstanding the backlog queue to be a multiprocessing queue not an inprocess queue Queue

[benclifford]

this needs updating against master (now that #3794 is merged)

[benclifford]

see other comment about test hang

[benclifford]

This dropped out of the merge queue due to a hang again. The first time it dropped out, I investigated and it looked like a multiprocessing related monitoring problem, unrelated to this PR.

The second time it dropped out, I have looked at the logs for that failed test and it looks like the hang is inside an MPI executor test, so I would like to review that a bit more seriously.

The run directory is runinfo/008 inside https://github.com/Parsl/parsl/actions/runs/13796405061/artifacts/2733031351

As an example of suspicious behaviour: htex task 106 is placed into the backlog in manager.log but no worker ever executes that task:

runinfo/008/MPI_TEST/block-0/9a0f7d3b72d8$ grep 106 worker_*.log

returns nothing.

I haven't audited the other tasks in that log for liveness.

If it is only the last task failing, then maybe a race condition around final task handling ("more tasks need to keep arriving to keep backlog tasks scheduled" or something like that?). I have also not investigated the code in depth to look at that.

I've dropped off my approval for this PR until more eyes look at this case.

[benclifford]

I'll note that the final task completion is logged in the same millisecond as the final backlog placement:

2025-03-11 19:34:46.798 parsl.executors.high_throughput.mpi_resource_management:179 24205 Task-Puller [INFO]  Not enough resources, placing task 106 into backlog

2025-03-11 19:34:46.798 worker_log:764 24220 MainThread [INFO]  All processing finished for executor task 105

My gut then says theres a race condition:
on the task submit side:

T1: decide a task needs to be backlogged
T3: place the task in the backlog

on the results side:
T2: return from task 105, see that backlog is empty, do not schedule empty backlog

and now no event ever happens to schedule the backlog.

I have not really dug into this, and I have not looked at code prior to the PR to see if I get the same gut feeling there.