Discovery domain does not reschedule lazily executed Tasks

[amydevs]

Describe the bug

The Discovery domain does not reschedule lazily executed Tasks when stopping. The trace for this is as follows:

1. Discovery.start is called to register handlers onto TaskManager before it starts processing.
2. TaskManager.startProcessing is called to start task processing.
3. The Discovery.discoverVertexHandler is executed lazily.
4. TaskManager.stopProcessing is called in order to stop task processing.
5. Discovery.discoverVertexHandler catches an ErrorTaskStop on ctx.signal.reason.
6. It calls Discovery.scheduleDiscoveryForVertex to reschedule the task.
7. Discovery.scheduleDiscoveryForVertex calls TaskManager.getTasks, seeing that the task that is currently executing cleanup already exists.
8. The new task is not rescheduled.
9. TaskManager.startProcessing is called, the task to resume discovery is not executed.

To Reproduce

As the code to reproduce this only exists within the context of my Discovery domain pagination changes, please refer to

Polykey/tests/discovery/Discovery.test.ts

Line 494 in 167b041

test('identity discovery persistence across restarts', async () => {

Expected behavior

The discovery task should be rescheduled so that discovery of vertices can be resumed after restarts.

Screenshots

Platform (please complete the following information)

• Device: [e.g. iPhone6]
• OS: [e.g. iOS]
• Version [e.g. 22]

Additional context

Found whilst working on #716

Notify maintainers

@tegefaulkes

[linear]

ENG-319 `Discovery` domain does not reschedule lazily executed `Task`s

[amydevs]

there needs to be a way to override the scheduleDiscoveryForVertex i think so that we force rescheduling, or just have a different method altogether

[tegefaulkes]

From what I recall from coding this bit, The intention was for there to only be 1 task per vertex. This was enforced in 2 ways.

1. When scheduling a vertex if there was a task that already exists for it then that task was modified instead of creating a new one.
2. If there just happens to be multiple tasks for a vertex, the extra tasks are cancelled.

At the time rescheduling just wasn't implemented and this was the intended behaviour. So I'm not really sure this can be considered a bug. That said, the requirements have changed and we need the ability to have a task reschedule itself if it doesn't fully complete. To that end we can look into reducing or removing these restrictions but we need to be careful about it.

[tegefaulkes]

I think the main issue right now is that the scheduleDiscoveryForVertex will not schedule a new task if there is an already scheduled task. But an active task is technically scheduled task. so calling scheduleDiscoveryForVertex inside the discoverVertex` handler will just skip the task since it's active.

One way to address this is to allow scheduleDiscoveryForVertex to schedule multiple discoverVertex tasks, Basically if there is any active tasks it will allow 1 new task to be scheduled. Or basically basically, just make it ignore active tasks.