Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use spawn_blocking for parsing #5582

Merged
merged 22 commits into from
Jul 9, 2024
Merged

use spawn_blocking for parsing #5582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
67c9361
use std mutex for query analysis cache, use spawn_blocking for parsing
May 24, 2024
98908a1
changeset
May 24, 2024
8e5e47f
fmt
May 24, 2024
5ce278e
Merge branch 'dev' into spawn-blocking-parser
xuorig May 24, 2024
70f1f1b
dont change mutex yet
May 24, 2024
b4625cc
refactor, spawn_blocking in warm up as well
May 28, 2024
df8a6b6
lint
May 29, 2024
1aee208
Merge branch 'dev' into spawn-blocking-parser
xuorig May 29, 2024
128c55c
Merge branch 'dev' into spawn-blocking-parser
xuorig May 30, 2024
a7f1cc2
Merge branch 'dev' into spawn-blocking-parser
xuorig Jun 4, 2024
9e6ea3e
Merge branch 'dev' into spawn-blocking-parser
xuorig Jun 25, 2024
c3493e9
lint
Jun 25, 2024
1886e58
Merge branch 'dev' into spawn-blocking-parser
Geal Jul 2, 2024
1c6df83
update snapshots
Geal Jul 2, 2024
af60e73
Update .changesets/fix_spawn_blocking_parser.md
Geal Jul 2, 2024
6a994d5
Update .changesets/fix_spawn_blocking_parser.md
Geal Jul 2, 2024
f0fed76
check snapshots again
Geal Jul 2, 2024
8836e0e
Changing span processing from sync to async requires special handling
garypen Jul 9, 2024
0c5d696
revert snapshot changes
garypen Jul 9, 2024
b1389e3
Merge branch 'dev' into spawn-blocking-parser
garypen Jul 9, 2024
f08769f
Try to cleanup snapshots
garypen Jul 9, 2024
996914c
Another try at fixing the snapshot inconsistency
garypen Jul 9, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
48 changes: 48 additions & 0 deletions apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,54 @@ resourceSpans:
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
value:
stringValue: "[redacted]"
- key: apollo.client.uname
value:
stringValue: "[redacted]"
- key: apollo.graph.ref
value:
stringValue: test
- key: apollo.router.id
value:
stringValue: "[redacted]"
- key: apollo.schema.id
value:
stringValue: "[redacted]"
- key: apollo.user.agent
value:
stringValue: "[redacted]"
droppedAttributesCount: 0
scopeSpans:
- scope:
name: apollo-router
version: "[version]"
attributes: []
droppedAttributesCount: 0
spans:
- traceId: "[trace_id]"
spanId: "[span_id]"
traceState: ""
parentSpanId: "[span_id]"
flags: 0
name: parse_query
kind: 1
startTimeUnixNano: "[start_time]"
endTimeUnixNano: "[end_time]"
attributes: []
droppedAttributesCount: 0
events: []
droppedEventsCount: 0
links: []
droppedLinksCount: 0
status:
message: ""
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
Expand Down
48 changes: 48 additions & 0 deletions apollo-router/tests/snapshots/apollo_otel_traces__batch_send_header.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,54 @@ resourceSpans:
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
value:
stringValue: "[redacted]"
- key: apollo.client.uname
value:
stringValue: "[redacted]"
- key: apollo.graph.ref
value:
stringValue: test
- key: apollo.router.id
value:
stringValue: "[redacted]"
- key: apollo.schema.id
value:
stringValue: "[redacted]"
- key: apollo.user.agent
value:
stringValue: "[redacted]"
droppedAttributesCount: 0
scopeSpans:
- scope:
name: apollo-router
version: "[version]"
attributes: []
droppedAttributesCount: 0
spans:
- traceId: "[trace_id]"
spanId: "[span_id]"
traceState: ""
parentSpanId: "[span_id]"
flags: 0
name: parse_query
kind: 1
startTimeUnixNano: "[start_time]"
endTimeUnixNano: "[end_time]"
attributes: []
droppedAttributesCount: 0
events: []
droppedEventsCount: 0
links: []
droppedLinksCount: 0
status:
message: ""
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
Expand Down
48 changes: 48 additions & 0 deletions apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,54 @@ resourceSpans:
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
value:
stringValue: "[redacted]"
- key: apollo.client.uname
value:
stringValue: "[redacted]"
- key: apollo.graph.ref
value:
stringValue: test
- key: apollo.router.id
value:
stringValue: "[redacted]"
- key: apollo.schema.id
value:
stringValue: "[redacted]"
- key: apollo.user.agent
value:
stringValue: "[redacted]"
droppedAttributesCount: 0
scopeSpans:
- scope:
name: apollo-router
version: "[version]"
attributes: []
droppedAttributesCount: 0
spans:
- traceId: "[trace_id]"
spanId: "[span_id]"
traceState: ""
parentSpanId: "[span_id]"
flags: 0
name: parse_query
kind: 1
startTimeUnixNano: "[start_time]"
endTimeUnixNano: "[end_time]"
attributes: []
droppedAttributesCount: 0
events: []
droppedEventsCount: 0
links: []
droppedLinksCount: 0
status:
message: ""
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
Expand Down
48 changes: 48 additions & 0 deletions apollo-router/tests/snapshots/apollo_otel_traces__batch_trace_id.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,54 @@ resourceSpans:
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
value:
stringValue: "[redacted]"
- key: apollo.client.uname
value:
stringValue: "[redacted]"
- key: apollo.graph.ref
value:
stringValue: test
- key: apollo.router.id
value:
stringValue: "[redacted]"
- key: apollo.schema.id
value:
stringValue: "[redacted]"
- key: apollo.user.agent
value:
stringValue: "[redacted]"
droppedAttributesCount: 0
scopeSpans:
- scope:
name: apollo-router
version: "[version]"
attributes: []
droppedAttributesCount: 0
spans:
- traceId: "[trace_id]"
spanId: "[span_id]"
traceState: ""
parentSpanId: "[span_id]"
flags: 0
name: parse_query
kind: 1
startTimeUnixNano: "[start_time]"
endTimeUnixNano: "[end_time]"
attributes: []
droppedAttributesCount: 0
events: []
droppedEventsCount: 0
links: []
droppedLinksCount: 0
status:
message: ""
code: 0
schemaUrl: ""
schemaUrl: ""
- resource:
attributes:
- key: apollo.client.host
Expand Down
Loading