Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changesets/fix_spawn_blocking_parser.md
Original file line number Diff line number Diff line change
@@ -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
10 changes: 8 additions & 2 deletions apollo-router/src/query_planner/caching_query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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));
Expand Down
28 changes: 24 additions & 4 deletions apollo-router/src/services/layers/query_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ParsedDocument, SpecError> {
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(
Expand Down Expand Up @@ -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 {
Expand Down