diff --git a/.changesets/fix_renee_max_evaluated_plans_rs.md b/.changesets/fix_renee_max_evaluated_plans_rs.md new file mode 100644 index 0000000000..67f7b8d0f6 --- /dev/null +++ b/.changesets/fix_renee_max_evaluated_plans_rs.md @@ -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 \ No newline at end of file diff --git a/.circleci/config.yml b/.circleci/config.yml index 126c85a25d..50d08f1d50 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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. diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 2df5ad8c62..50371fd5dd 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -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; @@ -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")); + + 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, + max_evaluated_plans, + paths_limit: self.supergraph.query_planning.experimental_paths_limit, + }, } } } diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index cc058b5555..dac65efe45 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -166,20 +166,7 @@ impl PlannerMode { schema: &Schema, configuration: &Configuration, ) -> Result, 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 { diff --git a/apollo-router/tests/integration/fixtures/query_planner_max_evaluated_plans.graphql b/apollo-router/tests/integration/fixtures/query_planner_max_evaluated_plans.graphql new file mode 100644 index 0000000000..a1f388ed61 --- /dev/null +++ b/apollo-router/tests/integration/fixtures/query_planner_max_evaluated_plans.graphql @@ -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 +} diff --git a/apollo-router/tests/integration/query_planner.rs b/apollo-router/tests/integration/query_planner.rs index 4b1c1ab70e..5bf0ca799c 100644 --- a/apollo-router/tests/integration/query_planner.rs +++ b/apollo-router/tests/integration/query_planner.rs @@ -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"; diff --git a/apollo-router/tests/integration/query_planner/max_evaluated_plans.rs b/apollo-router/tests/integration/query_planner/max_evaluated_plans.rs new file mode 100644 index 0000000000..d6474aa30b --- /dev/null +++ b/apollo-router/tests/integration/query_planner/max_evaluated_plans.rs @@ -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; +}