Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable #6746

SimonSapin · 2025-02-07T11:01:58Z

Router creates a thread pool dedicated to "compute jobs" to avoid blocking Tokio threads. By default this pool has as many threads as available CPUs. This can now be changed by setting the $APOLLO_ROUTER_COMPUTE_THREADS environment variable to an integer value. This feature is intentionally undocumented and should be considered "experimental".

Also rename the pre-existing (also undocumented) $APOLLO_ROUTER_NUM_CORES to $APOLLO_ROUTER_IO_THREADS to better reflect what it does.

Drive-by unrelated change: make cargo run default to the main router executable.

Manual testing instructions: in a terminal, run:

APOLLO_ROUTER_COMPUTE_THREADS=4 cargo run -- \
  -c router.yaml -s examples/graphql/supergraph.graphql --dev \
  --log error,apollo_router=info,apollo_router::compute_job=debug

After seeing a log line with GraphQL endpoint exposed at http://127.0.0.1:4000/, in another terminal, run:

curl -s -H 'accept: application/json' -H 'content-type: application/json' 'http://127.0.0.1:4000/' -d '{"query":"{__typename}"}'

(This step is needed because the thread pool is created lazily on first use.) The first terminal should now have a log line with Compute thread pool size: $APOLLO_ROUTER_COMPUTE_THREADS = 4.

Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

Changes are compatible¹
- Undocumented $APOLLO_ROUTER_NUM_CORES no longer works
Documentation² completed
Performance impact assessed and acceptable
Tests added and passing³
- Unit Tests
- Integration Tests
- Manual Tests

Exceptions

Note any exceptions here

Notes

It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩

Router creates a thread pool dedicated to "compute jobs", separate from Tokio threads. By default this pool has as many threads as available CPUs. This can now be changed by setting this new variable to an integer value. This feature is intentionally undocumented and should be considered "experimental". Also rename the pre-existing (also undocumented) $APOLLO_ROUTER_NUM_CORES to $APOLLO_ROUTER_IO_THREADS to better reflect what it does. Drive-by unrelated change: make `cargo run` default to the main router executable.

github-actions · 2025-02-07T11:02:13Z

@SimonSapin, please consider creating a changeset entry in /.changesets/. These instructions describe the process and tooling.

router-perf · 2025-02-07T11:02:30Z

CI performance tests

svc-apollo-docs · 2025-02-07T11:02:42Z

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 2cde78877e90d97611812265

apollo-router/Cargo.toml

SimonSapin · 2025-02-07T11:05:23Z

apollo-router/src/compute_job.rs

+        let size = std::thread::available_parallelism()
+            .expect("available_parallelism() failed")
+            .get();
+        tracing::debug!("Compute thread pool size: available_parallelism() = {size}");


This log can also help makes sure cgroup limits affect available_parallelism() like we expect when benchmarking

tracing::info!(size, type="compute", source="calculated", "thread pool");

garypen

Formatting up strings in logs makes them harder to consume or work with from JSON logs.

garypen · 2025-02-10T13:45:33Z

apollo-router/src/compute_job.rs

+        .ok()
+        .and_then(|value| value.parse::<usize>().ok())
+    {
+        tracing::debug!("Compute thread pool size: $APOLLO_ROUTER_COMPUTE_THREADS = {size}");


Better format for JSON parseability:

tracing::info!(size, type="compute", source="env", "thread pool");

I'd make it info since it will not be flooding the logs and it seems like useful information

garypen · 2025-02-10T13:45:42Z

apollo-router/src/compute_job.rs

+        let size = std::thread::available_parallelism()
+            .expect("available_parallelism() failed")
+            .get();
+        tracing::debug!("Compute thread pool size: available_parallelism() = {size}");


tracing::info!(size, type="compute", source="calculated", "thread pool");

garypen · 2025-02-10T13:47:44Z

apollo-router/src/executable.rs

+
+    // This environment variable is intentionally undocumented.
+    // See also APOLLO_ROUTER_COMPUTE_THREADS in apollo-router/src/compute_job.rs
+    if let Some(nb) = std::env::var("APOLLO_ROUTER_IO_THREADS")
        .ok()
        .and_then(|value| value.parse::<usize>().ok())
    {
        builder.worker_threads(nb);


Worth logging these details as well:

tracing::info!(size=nb, type="io", source="env", "thread pool");

Can tracing be used before we create a Tokio runtime? (If not, I can move things around to log after we’re in async context)

When I added this log it was captured in the snapshot for test pq_layer_freeform_graphql_with_safelist_log_unknown_true which made the outcome of the test dependent of number of available CPUs. Rather than spend time on figuring out how to make that test filter out that specific log I’ve removed the log for now.

SimonSapin · 2025-02-10T15:17:00Z

apollo-router/src/executable.rs

@@ -677,6 +681,11 @@ impl Executable {
            }
        };

+        let threads = tokio::runtime::Handle::current().metrics().num_workers();
+        tracing::info!(threads, type="io", "thread pool");


This is done here after we’ve initialized enough of the logging infrastructure. Unlike for compute threads there is no source here. The source could be:

$APOLLO_ROUTER_IO_THREADS, if specified and apollo_router::executable::main is used

Anything, if a custom binary defines its own main creating a Tokio runtime

$TOKIO_WORKER_THREADS if specified and nothing set Tokio’s worker_threads()

available_parallelism() is the eventual default

garypen · 2025-02-11T11:49:13Z

apollo-router/src/compute_job.rs

-        .expect("available_parallelism() failed")
-        .get()
+    // This environment variable is intentionally undocumented.
+    // See also APOLLO_ROUTER_IO_THREADS in apollo-router/src/executable.rs


Suggested change

// See also APOLLO_ROUTER_IO_THREADS in apollo-router/src/executable.rs

garypen · 2025-02-11T11:49:52Z

apollo-router/src/compute_job.rs

 }

 type Job = Box<dyn FnOnce() + Send + 'static>;

-fn queue() -> &'static AgeingPriorityQueue<Job> {
+pub(crate) fn queue() -> &'static AgeingPriorityQueue<Job> {


Deos this need to be pub(crate) now?

Good points!

lrlna · 2025-02-11T13:21:10Z

apollo-router/src/compute_job.rs

+        tracing::info!(threads, type="compute", source="env", "thread pool");
+        threads
+    } else {
+        let threads = std::thread::available_parallelism()
+            .expect("available_parallelism() failed")
+            .get();
+        tracing::info!(threads, type="compute", source="available_parallelism", "thread pool");
+        threads


should we emit a metric for the number of threads available and the number of threads being set by the env var?

(cherry picked from commit 768f6d4)

…) (#6762)

SimonSapin requested review from a team as code owners February 7, 2025 11:01

SimonSapin commented Feb 7, 2025

View reviewed changes

apollo-router/Cargo.toml Show resolved Hide resolved

SimonSapin added the backport-1.x label Feb 7, 2025

SimonSapin commented Feb 7, 2025

View reviewed changes

goto-bus-stop mentioned this pull request Feb 7, 2025

Add experimental non-reloadable config for worker pool size #6523

Closed

6 tasks

garypen reviewed Feb 10, 2025

View reviewed changes

Structured logging

Loading
Loading status checks…

6daa6f1

SimonSapin commented Feb 10, 2025

View reviewed changes

SimonSapin added 3 commits February 10, 2025 16:39

snap

Loading
Loading status checks…

42bec40

Revert IO thread pool size logging

94acf58

Merge branch 'dev' into simon/ROUTER-951

2f7a094

garypen self-requested a review February 11, 2025 11:48

garypen reviewed Feb 11, 2025

View reviewed changes

Review comments

a7179c0

garypen approved these changes Feb 11, 2025

View reviewed changes

SimonSapin enabled auto-merge (squash) February 11, 2025 11:52

lrlna reviewed Feb 11, 2025

View reviewed changes

Also revert logging for compute thread count

b9acd4d

SimonSapin merged commit 768f6d4 into dev Feb 11, 2025
15 checks passed

SimonSapin deleted the simon/ROUTER-951 branch February 11, 2025 14:43

SimonSapin added a commit that referenced this pull request Feb 11, 2025

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable (#6746)

b2794b4

(cherry picked from commit 768f6d4)

SimonSapin mentioned this pull request Feb 11, 2025

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable (backport #6746) #6762

Merged

6 tasks

SimonSapin added a commit that referenced this pull request Feb 11, 2025

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable (backport #6746…

98bd9e3

…) (#6762)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable #6746

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable #6746

SimonSapin commented Feb 7, 2025

github-actions bot commented Feb 7, 2025

router-perf bot commented Feb 7, 2025

svc-apollo-docs commented Feb 7, 2025 •

edited

Loading

SimonSapin Feb 7, 2025

garypen Feb 10, 2025

garypen left a comment •

edited

Loading

garypen Feb 10, 2025

SimonSapin Feb 10, 2025

garypen Feb 10, 2025

garypen Feb 10, 2025

SimonSapin Feb 10, 2025

SimonSapin Feb 11, 2025

SimonSapin Feb 10, 2025

garypen Feb 11, 2025

garypen Feb 11, 2025

SimonSapin Feb 11, 2025

lrlna Feb 11, 2025

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable #6746

Add $APOLLO_ROUTER_COMPUTE_THREADS environment variable #6746

Conversation

SimonSapin commented Feb 7, 2025

Footnotes

github-actions bot commented Feb 7, 2025

router-perf bot commented Feb 7, 2025

svc-apollo-docs commented Feb 7, 2025 • edited Loading

✅ Docs preview has no changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garypen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svc-apollo-docs commented Feb 7, 2025 •

edited

Loading

garypen left a comment •

edited

Loading