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.

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

fix: propagate evaluated plans limit and paths limit to the native query planner #6316

Merged
merged 7 commits into from
Dec 2, 2024
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
7 changes: 7 additions & 0 deletions .changesets/fix_renee_max_evaluated_plans_rs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
### fix: propagate evaluated plans limit and paths limit to the native query planner ([PR #6316](https://github.com/apollographql/router/pull/6316))

Two experimental query planning complexity limiting options now work with the native query planner:
- `supergraph.query_planning.experimental_plans_limit`
- `supergraph.query_planning.experimental_paths_limit`

By [@goto-bus-stop](https://github.com/goto-bus-stop) in https://github.com/apollographql/router/pull/6316
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
version: 2.1

# Cache key bump: 2
# Cache key bump: 3

# These "CircleCI Orbs" are reusable bits of configuration that can be shared
# across projects. See https://circleci.com/orbs/ for more information.
Expand Down
28 changes: 22 additions & 6 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::io::BufReader;
use std::iter;
use std::net::IpAddr;
use std::net::SocketAddr;
use std::num::NonZeroU32;
use std::num::NonZeroUsize;
use std::str::FromStr;
use std::sync::Arc;
Expand Down Expand Up @@ -416,16 +417,31 @@ impl Configuration {
pub(crate) fn rust_query_planner_config(
&self,
) -> apollo_federation::query_plan::query_planner::QueryPlannerConfig {
apollo_federation::query_plan::query_planner::QueryPlannerConfig {
use apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig;
use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
use apollo_federation::query_plan::query_planner::QueryPlannerDebugConfig;

let max_evaluated_plans = self
.supergraph
.query_planning
.experimental_plans_limit
// Fails if experimental_plans_limit is zero; use our default.
.and_then(NonZeroU32::new)
.unwrap_or(NonZeroU32::new(10_000).expect("it is not zero"));
lrlna marked this conversation as resolved.
Show resolved Hide resolved

QueryPlannerConfig {
reuse_query_fragments: self.supergraph.reuse_query_fragments.unwrap_or(true),
subgraph_graphql_validation: false,
generate_query_fragments: self.supergraph.generate_query_fragments,
incremental_delivery:
apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig {
enable_defer: self.supergraph.defer_support,
},
incremental_delivery: QueryPlanIncrementalDeliveryConfig {
enable_defer: self.supergraph.defer_support,
},
type_conditioned_fetching: self.experimental_type_conditioned_fetching,
debug: Default::default(),
debug: QueryPlannerDebugConfig {
bypass_planner_for_single_subgraph: false,
goto-bus-stop marked this conversation as resolved.
Show resolved Hide resolved
max_evaluated_plans,
paths_limit: self.supergraph.query_planning.experimental_paths_limit,
},
}
}
}
Expand Down
15 changes: 1 addition & 14 deletions apollo-router/src/query_planner/bridge_query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,7 @@ impl PlannerMode {
schema: &Schema,
configuration: &Configuration,
) -> Result<Arc<QueryPlanner>, ServiceBuildError> {
let config = apollo_federation::query_plan::query_planner::QueryPlannerConfig {
reuse_query_fragments: configuration
.supergraph
.reuse_query_fragments
.unwrap_or(true),
subgraph_graphql_validation: false,
generate_query_fragments: configuration.supergraph.generate_query_fragments,
incremental_delivery:
apollo_federation::query_plan::query_planner::QueryPlanIncrementalDeliveryConfig {
enable_defer: configuration.supergraph.defer_support,
},
type_conditioned_fetching: configuration.experimental_type_conditioned_fetching,
debug: Default::default(),
};
let config = configuration.rust_query_planner_config();
let result = QueryPlanner::new(schema.federation_supergraph(), config);

match &result {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Composed from subgraphs with hash: a9236eee956ed7fc219b2212696478159ced7eea
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
@link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION)
{
query: Query
}

directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION

directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE

directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION

directive @join__graph(name: String!, url: String!) on ENUM_VALUE

directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE

directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR

directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION

directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA

input join__ContextArgument {
name: String!
type: String!
context: String!
selection: join__FieldValue!
}

scalar join__DirectiveArguments

scalar join__FieldSet

scalar join__FieldValue

enum join__Graph {
SUBGRAPH1 @join__graph(name: "Subgraph1", url: "none")
SUBGRAPH2 @join__graph(name: "Subgraph2", url: "none")
}

scalar link__Import

enum link__Purpose {
"""
`SECURITY` features provide metadata necessary to securely resolve fields.
"""
SECURITY

"""
`EXECUTION` features provide metadata necessary for operation execution.
"""
EXECUTION
}

type Query
@join__type(graph: SUBGRAPH1)
@join__type(graph: SUBGRAPH2)
{
t: T
}

type T
@join__type(graph: SUBGRAPH1, key: "id")
@join__type(graph: SUBGRAPH2, key: "id")
{
id: ID!
v1: Int
v2: Int
v3: Int
v4: Int
}
2 changes: 2 additions & 0 deletions apollo-router/tests/integration/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use std::path::PathBuf;
use crate::integration::common::graph_os_enabled;
use crate::integration::IntegrationTest;

mod max_evaluated_plans;

const PROMETHEUS_METRICS_CONFIG: &str = include_str!("telemetry/fixtures/prometheus.router.yaml");
const LEGACY_QP: &str = "experimental_query_planner_mode: legacy";
const NEW_QP: &str = "experimental_query_planner_mode: new";
Expand Down
130 changes: 130 additions & 0 deletions apollo-router/tests/integration/query_planner/max_evaluated_plans.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
use serde_json::json;

use crate::integration::IntegrationTest;

fn assert_evaluated_plans(prom: &str, expected: u64) {
let line = prom
.lines()
.find(|line| line.starts_with("apollo_router_query_planning_plan_evaluated_plans_sum"))
.expect("apollo.router.query_planning.plan.evaluated_plans metric is missing");
let (_, num) = line
.split_once(' ')
.expect("apollo.router.query_planning.plan.evaluated_plans metric has unexpected shape");
assert_eq!(num, format!("{expected}"));
}

#[tokio::test(flavor = "multi_thread")]
async fn reports_evaluated_plans() {
let mut router = IntegrationTest::builder()
.config(
r#"
telemetry:
exporters:
metrics:
prometheus:
enabled: true
"#,
)
.supergraph("tests/integration/fixtures/query_planner_max_evaluated_plans.graphql")
.build()
.await;
router.start().await;
router.assert_started().await;
router
.execute_query(&json!({
"query": r#"{ t { v1 v2 v3 v4 } }"#,
"variables": {},
}))
.await;

let metrics = router
.get_metrics_response()
.await
.expect("failed to fetch metrics")
.text()
.await
.expect("metrics are not text?!");
assert_evaluated_plans(&metrics, 16);

router.graceful_shutdown().await;
}

#[tokio::test(flavor = "multi_thread")]
async fn does_not_exceed_max_evaluated_plans_legacy() {
let mut router = IntegrationTest::builder()
.config(
r#"
experimental_query_planner_mode: legacy
telemetry:
exporters:
metrics:
prometheus:
enabled: true
supergraph:
query_planning:
experimental_plans_limit: 4
"#,
)
.supergraph("tests/integration/fixtures/query_planner_max_evaluated_plans.graphql")
.build()
.await;
router.start().await;
router.assert_started().await;
router
.execute_query(&json!({
"query": r#"{ t { v1 v2 v3 v4 } }"#,
"variables": {},
}))
.await;

let metrics = router
.get_metrics_response()
.await
.expect("failed to fetch metrics")
.text()
.await
.expect("metrics are not text?!");
assert_evaluated_plans(&metrics, 4);

router.graceful_shutdown().await;
}

#[tokio::test(flavor = "multi_thread")]
async fn does_not_exceed_max_evaluated_plans() {
let mut router = IntegrationTest::builder()
.config(
r#"
experimental_query_planner_mode: new
telemetry:
exporters:
metrics:
prometheus:
enabled: true
supergraph:
query_planning:
experimental_plans_limit: 4
"#,
)
.supergraph("tests/integration/fixtures/query_planner_max_evaluated_plans.graphql")
.build()
.await;
router.start().await;
router.assert_started().await;
router
.execute_query(&json!({
"query": r#"{ t { v1 v2 v3 v4 } }"#,
"variables": {},
}))
.await;

let metrics = router
.get_metrics_response()
.await
.expect("failed to fetch metrics")
.text()
.await
.expect("metrics are not text?!");
assert_evaluated_plans(&metrics, 4);

router.graceful_shutdown().await;
}