diff --git a/.changesets/fix_spawn_blocking_parser.md b/.changesets/fix_spawn_blocking_parser.md new file mode 100644 index 0000000000..0fd442df94 --- /dev/null +++ b/.changesets/fix_spawn_blocking_parser.md @@ -0,0 +1,5 @@ +### use spawn_blocking for query parsing & validation ([PR #5235](https://github.com/apollographql/router/pull/5235)) + +Moves query parsing and validation in a tokio blocking task to prevent all executor threads from blocking on large queries. + +By [@xuorig](https://github.com/xuorig) in https://github.com/apollographql/router/pull/5235 \ No newline at end of file diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index ff28cbc4c5..a409df3756 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -269,7 +269,10 @@ where } in all_cache_keys { let context = Context::new(); - let doc = match query_analysis.parse_document(&query, operation.as_deref()) { + let doc = match query_analysis + .parse_document(&query, operation.as_deref()) + .await + { Ok(doc) => doc, Err(_) => continue, }; @@ -310,7 +313,10 @@ where }) .await; if entry.is_first() { - let doc = match query_analysis.parse_document(&query, operation.as_deref()) { + let doc = match query_analysis + .parse_document(&query, operation.as_deref()) + .await + { Ok(doc) => doc, Err(error) => { let e = Arc::new(QueryPlannerError::SpecError(error)); diff --git a/apollo-router/src/services/layers/query_analysis.rs b/apollo-router/src/services/layers/query_analysis.rs index 4beb5a1e21..71eae898e7 100644 --- a/apollo-router/src/services/layers/query_analysis.rs +++ b/apollo-router/src/services/layers/query_analysis.rs @@ -11,6 +11,7 @@ use http::StatusCode; use lru::LruCache; use router_bridge::planner::UsageReporting; use tokio::sync::Mutex; +use tokio::task; use crate::apollo_studio_interop::generate_extended_references; use crate::apollo_studio_interop::ExtendedReferenceStats; @@ -73,12 +74,32 @@ impl QueryAnalysisLayer { } } - pub(crate) fn parse_document( + pub(crate) async fn parse_document( &self, query: &str, operation_name: Option<&str>, ) -> Result { - Query::parse_document(query, operation_name, &self.schema, &self.configuration) + let query = query.to_string(); + let operation_name = operation_name.map(|o| o.to_string()); + let schema = self.schema.clone(); + let conf = self.configuration.clone(); + + // Must be created *outside* of the spawn_blocking or the span is not connected to the + // parent + let span = tracing::info_span!(QUERY_PARSING_SPAN_NAME, "otel.kind" = "INTERNAL"); + + task::spawn_blocking(move || { + span.in_scope(|| { + Query::parse_document( + &query, + operation_name.as_deref(), + schema.as_ref(), + conf.as_ref(), + ) + }) + }) + .await + .expect("parse_document task panicked") } pub(crate) async fn supergraph_request( @@ -127,8 +148,7 @@ impl QueryAnalysisLayer { let res = match entry { None => { - let span = tracing::info_span!(QUERY_PARSING_SPAN_NAME, "otel.kind" = "INTERNAL"); - match span.in_scope(|| self.parse_document(&query, op_name.as_deref())) { + match self.parse_document(&query, op_name.as_deref()).await { Err(errors) => { (*self.cache.lock().await).put( QueryAnalysisKey {