feat: queuing of calls to python process by doc-han · Pull Request #718 · OpenFn/apollo

doc-han · 2026-10-13T09:05:57Z

Short Description

Caps how many python child processes bridge.ts runs at once
(APOLLO_MAX_CONCURRENT_PYTHON, default 10). Requests past the cap queue in
FIFO order in Node instead of spawning immediately, so a burst of traffic
can't pile an unbounded number of python interpreters on top of each other.

Fixes #

Implementation Details

Today every request to a python service spawns (or forks, if
APOLLO_FORK_SERVER is on) a process immediately, with no limit on how many
run at once. A burst of concurrent requests has no backpressure before it
hits the host's CPU/memory limits.

platform/src/util/python-queue.ts (new) — a small hand-rolled FIFO
semaphore: acquireSlot(scriptName, signal) grants immediately while under
APOLLO_MAX_CONCURRENT_PYTHON, otherwise queues the caller. If signal
aborts while still queued (the client went away before its turn), the wait
rejects and the slot is never consumed - the request never spawns a process
at all. releaseSlot() hands a freed slot straight to the next waiter.
platform/src/bridge.ts - run() now calls acquireSlot() right
before doing any real work, and releases exactly once (guarded against
double-release) from whichever path ends the run first. This applies
uniformly to both execution paths: forking from the warm master
(USE_FORK_SERVER) and spawning a fresh interpreter - either way it's one
python process doing a service's worth of real work, so both are capped the
same way.
.env.example - documents APOLLO_MAX_CONCURRENT_PYTHON (default 10).

Testing:

platform/test/util/python-queue.test.ts - pure unit tests of the queue
logic: immediate grant under the cap, FIFO ordering past it, a queued wait
rejecting on abort without consuming a slot, and an already-aborted signal
rejecting immediately when the cap is full.
platform/test/bridge-queue.test.ts - real-subprocess integration tests via
the existing _cancel_probe fixture: two run() calls serialize under a
cap of 1, and a call cancelled while queued never spawns a process.
Verified live with bun dev: under APOLLO_MAX_CONCURRENT_PYTHON=1/2,
concurrent requests to /services/echo correctly queue
(queueing echo: 1/1 python processes running, 1 waiting) and complete
serially; repeated with APOLLO_FORK_SERVER=true to confirm the same
queueing holds for the fork path.
bun test platform/test: 122 pass, 7 fail (pre-existing, unrelated -
live-Postgres tests with no DB available locally).

AI Usage

Please disclose whether you've used AI in this work (it's cool, we just want to
know!):

Yes, I have used AI
No, I have not used AI

You can read more details in our
Responsible AI Policy

josephjclark · 2026-10-16T09:21:05Z

Joe to do: write a script to send a bunch of concurrent requests and see how the queue behaves

josephjclark

Just some initial thoughts, shooting from the hip

josephjclark · 2026-10-16T08:44:08Z

+# Max python child processes bridge.ts runs at once, per Apollo process.
+# Requests past this cap queue in FIFO order rather than spawning immediately.
+# Defaults to 10. See platform/src/util/python-queue.ts.
+# APOLLO_MAX_CONCURRENT_PYTHON=10


This isn't just python though, it's service calls in general. So APOLLO_MAX_CONCURRENT_SERVICE_CALLS is technically what this is, although that's super long.

Maybe APOLLO_CONCURRENCY? maybe claude has better suggestions. Just take python out of it please.

josephjclark · 2026-10-16T08:45:03Z

+// FIFO semaphore capping how many python processes bridge.ts runs at once.
+// Requests past the cap wait here instead of spawning.
+
+const DEFAULT_MAX_CONCURRENT = 10;


IS 10 a random number?

Do we have any idea of the memory overhead per fork?

josephjclark · 2026-10-16T08:49:06Z

+
+const DEFAULT_MAX_CONCURRENT = 10;
+
+export const maxConcurrentPython = (): number => {


I can't help but look at this and think "why is this so complicated?"

Can I suggest:

const DEFAULT_MAX_CONCURRENT = '10'; const maxConcurrency = Number(process.env.APOLLO_MAX_CONCURRENT_PYTHON ?? DEFAULT_MAX_CONCURRENT)

josephjclark · 2026-10-16T08:52:11Z

+    : DEFAULT_MAX_CONCURRENT;
+};
+
+let active = 0;


So this is module scoped. Only one queue per process.

Is that bad for testing? For re-use? Should there be a class or at least some instance based here?

Open question, maybe this is fine. Just sticks out at me a little as a weak architecture

josephjclark · 2026-10-16T08:53:31Z

+let active = 0;
+const queue: (() => void)[] = [];
+
+export const activeCount = (): number => active;


Are either of these actually neccessary? You could just export active and queue?

Looks like they're both just used by unit tests anyway

josephjclark · 2026-10-16T08:55:17Z

+
+/** Resolves once a slot is free, FIFO. Rejects without consuming a slot if
+ *  `signal` is already aborted or aborts while still queued. */
+export const acquireSlot = (


I personally don't like the language of this.

This is a queue implementation. Why is there no add or enqueue or pop API?

A slot is just a promise right? I'm sort of just expecting clients to push tasks on to the queue and get a callback or event when done.

This slot business feels... wierd to me.

If its the correct language for this sort of thing I stand corrected, but it's not what I expect

josephjclark · 2026-10-16T08:57:44Z

+    };
+    queue.push(grant);
+
+    signal?.addEventListener(


Should signal be mandatory?

josephjclark · 2026-10-16T08:59:32Z

+ *  `signal` is already aborted or aborts while still queued. */
+export const acquireSlot = (
+  scriptName: string,
+  signal?: AbortSignal


The signal stuff is new to be and I see it being passed a looong way down the stack.

If acquireSlot returned a { cancel } object, could we do the signal stuff higher up the stack? Would that help anything? Maybe not

josephjclark · 2026-10-16T09:24:22Z

+
+/** Releases a slot acquireSlot() granted, straight to the next waiter. Call
+ *  exactly once per resolved acquireSlot(). */
+export const releaseSlot = (): void => {


Should this be guarded to be a bit more idempotent? Or are we absolutely sure it's super tight?

Or what if every slot has its own release functuon, like with the cancel function?

josephjclark · 2026-10-16T09:25:49Z

    // Nothing was spawned, so no "close" is coming - without settling here the
    // request stays open until something upstream gives up
    proc.on("error", (err) => {
+      releaseSlotOnce();


the foked bridge doesn't do this - is that because it throws on error and we release in the finally block?

josephjclark

Oh and we mentioned raising a sentry alert whenever an item is added to the queue? Or perhaps if more than N items are added

josephjclark · 2026-10-16T09:39:16Z

+
+/** Resolves once a slot is free, FIFO. Rejects without consuming a slot if
+ *  `signal` is already aborted or aborts while still queued. */
+export const acquireSlot = (


Should there be some sort of timeout on this? Like if waiting for more than 5 minutes should it return a 503 or something?

doc-han changed the base branch from main to forked-python-process October 13, 2026 09:22

doc-han changed the base branch from forked-python-process to main October 14, 2026 10:56

feat: queuing of python process

7443f35

doc-han force-pushed the queue-python-process-calls branch from e7cb05e to 7443f35 Compare October 14, 2026 13:04

doc-han marked this pull request as ready for review October 14, 2026 13:30

chore: cleanup comments

150de1a

josephjclark reviewed Oct 16, 2026

View reviewed changes


		const DEFAULT_MAX_CONCURRENT = 10;

		export const maxConcurrentPython = (): number => {

Conversation

doc-han commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Short Description

Implementation Details

AI Usage

Uh oh!

josephjclark commented Oct 16, 2026

Uh oh!

josephjclark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephjclark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

doc-han commented Oct 13, 2026 •

edited

Loading