From c91e6610cfd8abfe8b11fd4b93959e313c469152 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 28 Mar 2024 15:23:26 -0700 Subject: [PATCH 1/9] add generate_query_fragments configuration option --- Cargo.lock | 4 ++-- apollo-router/Cargo.toml | 2 +- apollo-router/src/configuration/mod.rs | 22 +++++++++++++++++++ apollo-router/src/configuration/tests.rs | 16 ++++++++++++++ apollo-router/src/introspection.rs | 1 + .../src/query_planner/bridge_query_planner.rs | 2 ++ 6 files changed, 44 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 186857ec6d..98cc04c42d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5670,9 +5670,9 @@ dependencies = [ [[package]] name = "router-bridge" -version = "0.5.16+v2.7.1" +version = "0.5.17+v2.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "224f69e11bdc5c16b582f82cd18647e305d31c9c20110635fcdf790c46405777" +checksum = "2f183e217179b38a4283e76ca62e3149ebe96512e9b1bd6b3933abab863f9a2c" dependencies = [ "anyhow", "async-channel 1.9.0", diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index d656c31166..c9ac2d4096 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -186,7 +186,7 @@ reqwest = { version = "0.11.24", default-features = false, features = [ "stream", ] } # note: this dependency should _always_ be pinned, prefix the version with an `=` -router-bridge = "=0.5.16+v2.7.1" +router-bridge = "=0.5.17+v2.7.2" rust-embed = "8.2.0" rustls = "0.21.10" rustls-native-certs = "0.6.3" diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index f03406804b..fac95098d8 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -610,6 +610,11 @@ pub(crate) struct Supergraph { #[serde(rename = "experimental_reuse_query_fragments")] pub(crate) reuse_query_fragments: Option, + /// Enable QP generation of fragments for subgraph requests + /// Default: false + #[serde(rename = "generate_query_fragments")] + pub(crate) generate_query_fragments: Option, + /// Set to false to disable defer support pub(crate) defer_support: bool, @@ -631,7 +636,21 @@ impl Supergraph { defer_support: Option, query_planning: Option, reuse_query_fragments: Option, + generate_query_fragments: Option, ) -> Self { + // reuse and generate query fragments are mutually exclusive options. If both + // are set and true, generate will be used and a warning will be + // emitted. + let reuse_query_fragments = if generate_query_fragments.is_some_and(|v| v) { + if reuse_query_fragments.is_some_and(|v| v) { + // warn the user that both are enabled and it's overridden + tracing::warn!("Both 'generate_query_fragments' and 'experimental_reuse_query_fragments' are explicitly enabled, 'experimental_reuse_query_fragments' will be overridden to false"); + } + Some(false) + } else { + reuse_query_fragments + }; + Self { listen: listen.unwrap_or_else(default_graphql_listen), path: path.unwrap_or_else(default_graphql_path), @@ -639,6 +658,7 @@ impl Supergraph { defer_support: defer_support.unwrap_or_else(default_defer_support), query_planning: query_planning.unwrap_or_default(), reuse_query_fragments, + generate_query_fragments, } } } @@ -654,6 +674,7 @@ impl Supergraph { defer_support: Option, query_planning: Option, reuse_query_fragments: Option, + generate_query_fragments: Option, ) -> Self { Self { listen: listen.unwrap_or_else(test_listen), @@ -662,6 +683,7 @@ impl Supergraph { defer_support: defer_support.unwrap_or_else(default_defer_support), query_planning: query_planning.unwrap_or_default(), reuse_query_fragments, + generate_query_fragments, } } } diff --git a/apollo-router/src/configuration/tests.rs b/apollo-router/src/configuration/tests.rs index 4b15fe66a5..80add080b2 100644 --- a/apollo-router/src/configuration/tests.rs +++ b/apollo-router/src/configuration/tests.rs @@ -1000,3 +1000,19 @@ fn find_struct_name(lines: &[&str], line_number: usize) -> Option { }) .next() } + +#[test] +fn it_prevents_reuse_and_generate_query_fragments_simultaneously() { + let conf = Configuration::builder() + .supergraph( + Supergraph::builder() + .generate_query_fragments(true) + .reuse_query_fragments(true) + .build(), + ) + .build() + .unwrap(); + + assert_eq!(conf.supergraph.generate_query_fragments, Some(true)); + assert_eq!(conf.supergraph.reuse_query_fragments, Some(false)); +} diff --git a/apollo-router/src/introspection.rs b/apollo-router/src/introspection.rs index d3af08418a..7cf4b6ca13 100644 --- a/apollo-router/src/introspection.rs +++ b/apollo-router/src/introspection.rs @@ -112,6 +112,7 @@ mod introspection_tests { }), graphql_validation: true, reuse_query_fragments: Some(false), + generate_query_fragments: None, debug: None, }, ) diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 8b32619933..c101d2afa0 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -96,6 +96,7 @@ impl BridgeQueryPlanner { sdl, QueryPlannerConfig { reuse_query_fragments: configuration.supergraph.reuse_query_fragments, + generate_query_fragments: configuration.supergraph.generate_query_fragments, incremental_delivery: Some(IncrementalDeliverySupport { enable_defer: Some(configuration.supergraph.defer_support), }), @@ -272,6 +273,7 @@ impl BridgeQueryPlanner { GraphQLValidationMode::Legacy | GraphQLValidationMode::Both ), reuse_query_fragments: configuration.supergraph.reuse_query_fragments, + generate_query_fragments: configuration.supergraph.generate_query_fragments, debug: Some(QueryPlannerDebugConfig { bypass_planner_for_single_subgraph: None, max_evaluated_plans: configuration From 3fa76701e68ea2efff86d5046dbac4d9bb46db16 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 28 Mar 2024 15:41:41 -0700 Subject: [PATCH 2/9] changeset --- .changesets/config_generate_query_fragments.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changesets/config_generate_query_fragments.md diff --git a/.changesets/config_generate_query_fragments.md b/.changesets/config_generate_query_fragments.md new file mode 100644 index 0000000000..2125b3d4d0 --- /dev/null +++ b/.changesets/config_generate_query_fragments.md @@ -0,0 +1,12 @@ +### Add `generate_query_fragments` configuration option ([PR #4885](https://github.com/apollographql/router/pull/4885)) + +Add a new `supergraph` configuration option `generate_query_fragments`. When set to `true`, the query planner will extract inline fragments into fragment definitions before sending queries to subgraphs. This can significantly reduce the size of the query sent to subgraphs, but may increase the time it takes to plan the query. Note that this option and `reuse_query_fragments` are mutually exclusive; if both are set to `true`, `generate_query_fragments` will take precedence. + +An example router configuration: + +```yaml title="router.yaml" +supergraph: + generate_query_fragments: true +``` + +By [@trevor-scheer](https://github.com/trevor-scheer) in https://github.com/apollographql/router/pull/4885 From 8cfdacea5ef9a74906b9151eb1500e1fed9f3f1f Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 29 Mar 2024 10:09:28 -0700 Subject: [PATCH 3/9] update snapshot --- ...o_router__configuration__tests__schema_generation.snap | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index f45def97f7..8ecf2885fd 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -1,5 +1,6 @@ --- source: apollo-router/src/configuration/tests.rs +assertion_line: 31 expression: "&schema" --- { @@ -2619,6 +2620,7 @@ expression: "&schema" "path": "/", "introspection": false, "experimental_reuse_query_fragments": null, + "generate_query_fragments": null, "defer_support": true, "query_planning": { "cache": { @@ -2657,6 +2659,12 @@ expression: "&schema" "type": "boolean", "nullable": true }, + "generate_query_fragments": { + "description": "Enable QP generation of fragments for subgraph requests Default: false", + "default": null, + "type": "boolean", + "nullable": true + }, "introspection": { "description": "Enable introspection Default: false", "default": false, From f15d5de589de935bd90fd5fdef23f2c5fef62a0c Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 29 Mar 2024 10:27:51 -0700 Subject: [PATCH 4/9] add docs --- docs/source/configuration/overview.mdx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/source/configuration/overview.mdx b/docs/source/configuration/overview.mdx index ce2e0d105d..8d9367c8df 100644 --- a/docs/source/configuration/overview.mdx +++ b/docs/source/configuration/overview.mdx @@ -850,6 +850,22 @@ example: password: "${env.MY_PASSWORD}" #highlight-line ``` +### Fragment reuse and generation + +By default, the Apollo Router will attempt to reuse fragments from the original query while forming subgraph requests. This behavior can be disabled by setting the option to `false`: + +```yaml +supergraph: + experimental_reuse_query_fragments: false +``` + +Alternatively, the Apollo Router can be configured to _generate_ fragments for subgraph requests. When set to `true`, the Apollo Router will extract _inline fragments only_ into fragment definitions before sending queries to subgraphs. This can significantly reduce the size of the query sent to subgraphs, but may increase the time it takes for planning. Note that this option and `experimental_reuse_query_fragments` are mutually exclusive; if both are explicitly set to `true`, `generate_query_fragments` will take precedence. + +```yaml +supergraph: + generate_query_fragments: true +``` + ### Reusing configuration You can reuse parts of your configuration file in multiple places using standard YAML aliasing syntax: From 4c5ef94cda795b3055513ef9dab21853608f2192 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 2 Apr 2024 16:19:14 -0700 Subject: [PATCH 5/9] generate shouldn't be an option, update tests --- apollo-router/src/configuration/mod.rs | 40 ++++++++++--------- ...nfiguration__tests__schema_generation.snap | 7 ++-- apollo-router/src/configuration/tests.rs | 4 +- .../src/query_planner/bridge_query_planner.rs | 4 +- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 4e8cf779c4..2b15df6236 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -612,8 +612,7 @@ pub(crate) struct Supergraph { /// Enable QP generation of fragments for subgraph requests /// Default: false - #[serde(rename = "generate_query_fragments")] - pub(crate) generate_query_fragments: Option, + pub(crate) generate_query_fragments: bool, /// Set to false to disable defer support pub(crate) defer_support: bool, @@ -650,27 +649,22 @@ impl Supergraph { early_cancel: Option, experimental_log_on_broken_pipe: Option, ) -> Self { - // reuse and generate query fragments are mutually exclusive options. If both - // are set and true, generate will be used and a warning will be - // emitted. - let reuse_query_fragments = if generate_query_fragments.is_some_and(|v| v) { - if reuse_query_fragments.is_some_and(|v| v) { - // warn the user that both are enabled and it's overridden - tracing::warn!("Both 'generate_query_fragments' and 'experimental_reuse_query_fragments' are explicitly enabled, 'experimental_reuse_query_fragments' will be overridden to false"); - } - Some(false) - } else { - reuse_query_fragments - }; - Self { listen: listen.unwrap_or_else(default_graphql_listen), path: path.unwrap_or_else(default_graphql_path), introspection: introspection.unwrap_or_else(default_graphql_introspection), defer_support: defer_support.unwrap_or_else(default_defer_support), query_planning: query_planning.unwrap_or_default(), - reuse_query_fragments, - generate_query_fragments, + reuse_query_fragments: generate_query_fragments.and_then(|v| + if v { + if reuse_query_fragments.is_some_and(|v| v) { + // warn the user that both are enabled and it's overridden + tracing::warn!("Both 'generate_query_fragments' and 'experimental_reuse_query_fragments' are explicitly enabled, 'experimental_reuse_query_fragments' will be overridden to false"); + } + Some(false) + } else { reuse_query_fragments } + ), + generate_query_fragments: generate_query_fragments.unwrap_or_default(), early_cancel: early_cancel.unwrap_or_default(), experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(), } @@ -698,8 +692,16 @@ impl Supergraph { introspection: introspection.unwrap_or_else(default_graphql_introspection), defer_support: defer_support.unwrap_or_else(default_defer_support), query_planning: query_planning.unwrap_or_default(), - reuse_query_fragments, - generate_query_fragments, + reuse_query_fragments: generate_query_fragments.and_then(|v| + if v { + if reuse_query_fragments.is_some_and(|v| v) { + // warn the user that both are enabled and it's overridden + tracing::warn!("Both 'generate_query_fragments' and 'experimental_reuse_query_fragments' are explicitly enabled, 'experimental_reuse_query_fragments' will be overridden to false"); + } + Some(false) + } else { reuse_query_fragments } + ), + generate_query_fragments: generate_query_fragments.unwrap_or_default(), early_cancel: early_cancel.unwrap_or_default(), experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(), } diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 17adcaceac..978243f186 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -2620,7 +2620,7 @@ expression: "&schema" "path": "/", "introspection": false, "experimental_reuse_query_fragments": null, - "generate_query_fragments": null, + "generate_query_fragments": false, "defer_support": true, "query_planning": { "cache": { @@ -2661,9 +2661,8 @@ expression: "&schema" }, "generate_query_fragments": { "description": "Enable QP generation of fragments for subgraph requests Default: false", - "default": null, - "type": "boolean", - "nullable": true + "default": false, + "type": "boolean" }, "introspection": { "description": "Enable introspection Default: false", diff --git a/apollo-router/src/configuration/tests.rs b/apollo-router/src/configuration/tests.rs index 80add080b2..81f38d8d6d 100644 --- a/apollo-router/src/configuration/tests.rs +++ b/apollo-router/src/configuration/tests.rs @@ -1013,6 +1013,6 @@ fn it_prevents_reuse_and_generate_query_fragments_simultaneously() { .build() .unwrap(); - assert_eq!(conf.supergraph.generate_query_fragments, Some(true)); - assert_eq!(conf.supergraph.reuse_query_fragments, Some(false)); + assert_eq!(conf.supergraph.generate_query_fragments, true); + assert_eq!(conf.supergraph.reuse_query_fragments, Some(true)); } diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 7935ad1a58..f1247d86c0 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -100,7 +100,7 @@ impl BridgeQueryPlanner { sdl, QueryPlannerConfig { reuse_query_fragments: configuration.supergraph.reuse_query_fragments, - generate_query_fragments: configuration.supergraph.generate_query_fragments, + generate_query_fragments: Some(configuration.supergraph.generate_query_fragments), incremental_delivery: Some(IncrementalDeliverySupport { enable_defer: Some(configuration.supergraph.defer_support), }), @@ -288,7 +288,7 @@ impl BridgeQueryPlanner { GraphQLValidationMode::Legacy | GraphQLValidationMode::Both ), reuse_query_fragments: configuration.supergraph.reuse_query_fragments, - generate_query_fragments: configuration.supergraph.generate_query_fragments, + generate_query_fragments: Some(configuration.supergraph.generate_query_fragments), debug: Some(QueryPlannerDebugConfig { bypass_planner_for_single_subgraph: None, max_evaluated_plans: configuration From accf541f777ad56efa3058ca8b8bdbfd5b69c833 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 2 Apr 2024 16:36:05 -0700 Subject: [PATCH 6/9] lint --- apollo-router/src/configuration/mod.rs | 5 ++++- apollo-router/src/configuration/tests.rs | 2 +- apollo-router/src/query_planner/bridge_query_planner.rs | 4 +++- apollo-router/tests/integration/mod.rs | 2 -- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 2b15df6236..c77a6b04ed 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -710,7 +710,10 @@ impl Supergraph { impl Default for Supergraph { fn default() -> Self { - Self::builder().build() + Self::builder() + .generate_query_fragments(false) + .reuse_query_fragments(true) + .build() } } diff --git a/apollo-router/src/configuration/tests.rs b/apollo-router/src/configuration/tests.rs index 81f38d8d6d..170e14c25c 100644 --- a/apollo-router/src/configuration/tests.rs +++ b/apollo-router/src/configuration/tests.rs @@ -1013,6 +1013,6 @@ fn it_prevents_reuse_and_generate_query_fragments_simultaneously() { .build() .unwrap(); - assert_eq!(conf.supergraph.generate_query_fragments, true); + assert!(conf.supergraph.generate_query_fragments); assert_eq!(conf.supergraph.reuse_query_fragments, Some(true)); } diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index f1247d86c0..64e61b8268 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -288,7 +288,9 @@ impl BridgeQueryPlanner { GraphQLValidationMode::Legacy | GraphQLValidationMode::Both ), reuse_query_fragments: configuration.supergraph.reuse_query_fragments, - generate_query_fragments: Some(configuration.supergraph.generate_query_fragments), + generate_query_fragments: Some( + configuration.supergraph.generate_query_fragments, + ), debug: Some(QueryPlannerDebugConfig { bypass_planner_for_single_subgraph: None, max_evaluated_plans: configuration diff --git a/apollo-router/tests/integration/mod.rs b/apollo-router/tests/integration/mod.rs index 2f2bcd9e0d..80ee7c18f5 100644 --- a/apollo-router/tests/integration/mod.rs +++ b/apollo-router/tests/integration/mod.rs @@ -1,8 +1,6 @@ #[path = "../common.rs"] pub(crate) mod common; pub(crate) use common::IntegrationTest; -pub(crate) use common::Telemetry; -pub(crate) use common::ValueExt; mod docs; mod file_upload; From faad9694b2a8ab90268b68fe32895ed155cdc8bd Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Wed, 3 Apr 2024 09:51:11 -0700 Subject: [PATCH 7/9] replace removed imports --- apollo-router/tests/integration/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apollo-router/tests/integration/mod.rs b/apollo-router/tests/integration/mod.rs index 80ee7c18f5..2f2bcd9e0d 100644 --- a/apollo-router/tests/integration/mod.rs +++ b/apollo-router/tests/integration/mod.rs @@ -1,6 +1,8 @@ #[path = "../common.rs"] pub(crate) mod common; pub(crate) use common::IntegrationTest; +pub(crate) use common::Telemetry; +pub(crate) use common::ValueExt; mod docs; mod file_upload; From d237b0523923d2a49c73d2e0fec96e1f1c168f03 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Wed, 3 Apr 2024 11:56:03 -0700 Subject: [PATCH 8/9] revert builder change and fix test --- apollo-router/src/configuration/mod.rs | 5 +---- apollo-router/src/configuration/tests.rs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index c77a6b04ed..2b15df6236 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -710,10 +710,7 @@ impl Supergraph { impl Default for Supergraph { fn default() -> Self { - Self::builder() - .generate_query_fragments(false) - .reuse_query_fragments(true) - .build() + Self::builder().build() } } diff --git a/apollo-router/src/configuration/tests.rs b/apollo-router/src/configuration/tests.rs index 170e14c25c..e8e985dec1 100644 --- a/apollo-router/src/configuration/tests.rs +++ b/apollo-router/src/configuration/tests.rs @@ -1014,5 +1014,5 @@ fn it_prevents_reuse_and_generate_query_fragments_simultaneously() { .unwrap(); assert!(conf.supergraph.generate_query_fragments); - assert_eq!(conf.supergraph.reuse_query_fragments, Some(true)); + assert_eq!(conf.supergraph.reuse_query_fragments, Some(false)); } From 7f9d8a816872ddd8f8e2bd4cc2258d9fa758951a Mon Sep 17 00:00:00 2001 From: bryn Date: Fri, 5 Apr 2024 12:48:29 +0100 Subject: [PATCH 9/9] Update redis key --- apollo-router/tests/integration/redis.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs index 08ef3e42f0..2c9b57159d 100644 --- a/apollo-router/tests/integration/redis.rs +++ b/apollo-router/tests/integration/redis.rs @@ -26,7 +26,7 @@ mod test { // 2. run `docker compose up -d` and connect to the redis container by running `docker exec -ti /bin/bash`. // 3. Run the `redis-cli` command from the shell and start the redis `monitor` command. // 4. Run this test and yank the updated cache key from the redis logs. - let known_cache_key = "plan:v2.7.1:af1ee357bc75cfbbcc6adda41089a56e7d1d52f6d44c049739dde2c259314f58:2bf7810d3a47b31d8a77ebb09cdc784a3f77306827dc55b06770030a858167c7"; + let known_cache_key = "plan:v2.7.2:af1ee357bc75cfbbcc6adda41089a56e7d1d52f6d44c049739dde2c259314f58:2bf7810d3a47b31d8a77ebb09cdc784a3f77306827dc55b06770030a858167c7"; let config = RedisConfig::from_url("redis://127.0.0.1:6379")?; let client = RedisClient::new(config, None, None, None);