Intermittent KeyError when handling a finished future in the task worker

[rtibbles]

Observed behavior

Occasionally, when a task is completed, deleting its future from the futures map results in a KeyError that is unhandled.

Errors and logs

ERROR    2023-10-04 15:52:05,784 exception calling callback for <Future at 0x116e39358 state=finished returned NoneType>
Traceback (most recent call last):
  File "/Users/runner/hostedtoolcache/Python/3.6.15/x64/lib/python3.6/concurrent/futures/_base.py", line 324, in _invoke_callbacks
    callback(self)
  File "/Users/runner/work/kolibri/kolibri/kolibri/core/tasks/worker.py", line 90, in handle_finished_future
    del self.future_job_mapping[job.job_id]
KeyError: '68a88bf36f0b4cd8942b4db6f895d6f5'

Expected behavior

Would be good to understand why these KeyErrors are happening, but ultimately making them either not happen, or be handled, because the deletion is unneeded is sufficient resolution here.

User-facing consequences

Erroneous error logs that suggest something has gone wrong when it hasn't

Steps to reproduce

Running the task runner tests in multiprocessing mode should be sufficient.

Context

Observed in the MacOS tests on Github Actions (but also occasionally observed locally with threaded task runners).

[laynestephens]

I would like to be assigned to this issue.

[Nyu10]

am also interested

[MisRob]

Hi @laynestephens and @Nyu10, thanks for volunteering. I will assign this to @laynestephens since they were first. @Nyu10 I assigned you another issue you asked for meanwhile.

[laynestephens]

Hi again! I am working with a partner on this issue, @adviti-mishra , and I was wondering if they could be assigned simultaneously.

[MisRob]

Hi @laynestephens, yes sure

[MisRob]

@laynestephens We will need @adviti-mishra to comment on this issue so I can assign them

[adviti-mishra]

Hello @MisRob Thank you so much! I'm with @laynestephens and would love to be assigned as well.

[adviti-mishra]

The KeyError issue seems to be in deleting a variable called future from self.job_future_mapping and a variable called job.job_id from self.future_job_mapping. The first fix that came to mind was adding an if check to see if the keys exist in the dictionaries before deleting them. However, we suspect the root cause of this error is a race condition where this function is trying to delete from the dictionaries in one thread while another function is trying to modify the dictionaries in another thread.
If our hypotheses are correct, could we make a PR for the first fix and then look into fixing the race condition? This is our first time contributing to open source, so we aren't sure if ideating like this on the issue is a good idea or not : ) Thank you!

[MisRob]

@adviti-mishra It seems you've already discussed this on Slack. Let us know there if you needed anything else. Thank you.

[rtibbles]

Fixed in #11591