Skip to content

Remove task-worker multiprocessing#14754

Merged
rtibbles merged 1 commit into
learningequality:developfrom
rtibbles:no_more_multiprocessing
Jun 4, 2026
Merged

Remove task-worker multiprocessing#14754
rtibbles merged 1 commit into
learningequality:developfrom
rtibbles:no_more_multiprocessing

Conversation

@rtibbles
Copy link
Copy Markdown
Member

Summary

  • Retires the in-process multiprocessing option for the async task runner. The worker becomes thread-only.
  • USE_WORKER_MULTIPROCESSING has defaulted to False for the lifetime of the option, the multiprocessing branches have never been production-ready, and the parametrized mock_compat fixture is a recurring source of flaky tests.
  • Deployers who want CPU isolation can run kolibri services in its own process.

References

  • No linked issue — internal cleanup driven by the recurring flaky test in multiprocessing mode, and the persistence of "zombie workers" during tests from improper shutdown.

Reviewer guidance

  • LogPlugin no longer attaches bus.log_queue. Worth confirming no out-of-tree plugin reads it off the magicbus.
  • options.ini files in the wild that still set USE_WORKER_MULTIPROCESSING = True continue to load — ConfigObj silently drops unknown keys. The setting just stops doing anything.

AI usage

Claude Code (Opus 4.7) handled this end-to-end: brainstormed the spec with me, wrote the implementation plan, dispatched subagents per plan task, and ran a self-review pass over the working tree against the spec. I reviewed the spec and plan before execution, deleted the unused conftest placeholder, and approved the commit structure.

@github-actions github-actions Bot added DEV: backend Python, databases, networking, filesystem... SIZE: medium labels May 22, 2026
@rtibbles rtibbles force-pushed the no_more_multiprocessing branch from 7286756 to ca351c4 Compare May 22, 2026 23:15
@rtibbles rtibbles marked this pull request as ready for review May 22, 2026 23:16
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

The in-process multiprocessing option for async task workers
(USE_WORKER_MULTIPROCESSING) has never been production-ready and
its parametrized test fixture is a source of flaky CI. Default has
been False for the life of the option.

The worker is now thread-only. Deployers who want CPU isolation can
run `kolibri services` as a separate OS process.

Changes:
- Delete kolibri/utils/multiprocessing_compat.py.
- Worker uses ThreadPoolExecutor directly; drop the log_queue
  parameter from execute_job, execute_job_with_python_worker,
  Worker, initialize_workers, and ServicesPlugin.START.
- utils.py: use threading.local / threading.Event directly; drop
  the dead ProcessPoolExecutor branch in fd_safe_executor.
- logger.py: switch the queue listener to stdlib queue.Queue; drop
  the pickle-safety and queue.close()/join_thread() branches that
  only mattered under multiprocessing; delete the unused
  setup_worker_logging bootstrap. The queue-based logging machinery
  itself stays (still useful on Windows).
- options.py: remove the USE_WORKER_MULTIPROCESSING schema entry
  and its now-orphaned multiprocess_bool validator. ConfigObj
  silently drops unknown keys, so existing options.ini files won't
  fail to load.
- Delete the parametrized mock_compat fixture in
  kolibri/core/tasks/test/taskrunner/conftest.py.
- Trim Job.task docstring to drop the multiprocessing reference.
Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a removal, the changes appear straightfoward and correct

@rtibbles rtibbles merged commit 32a581f into learningequality:develop Jun 4, 2026
59 checks passed
@rtibbles rtibbles deleted the no_more_multiprocessing branch June 4, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DEV: backend Python, databases, networking, filesystem... SIZE: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants