From 09325e20f2b7e37bea3e5810358e132c65878bd6 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Thu, 18 Aug 2022 16:11:04 +0100 Subject: [PATCH 1/2] rename two traffic shaping deduplication options New names are clearer. also: remove an old insta snapshot that is still checked into git. --- NEXT_CHANGELOG.md | 25 +++++++++++++++ ...nfiguration__tests__schema_generation.snap | 31 +++++++++---------- .../src/plugins/traffic_shaping/mod.rs | 18 +++++------ apollo-router/src/query_planner/mod.rs | 8 ++--- ...er__bridge_query_planner__tests__plan.snap | 20 ------------ docs/source/configuration/traffic-shaping.mdx | 6 ++-- 6 files changed, 56 insertions(+), 52 deletions(-) delete mode 100644 apollo-router/src/query_planner/snapshots/apollo_router_core__query_planner__bridge_query_planner__tests__plan.snap diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 912b349e3e..b3bfaf5a1d 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -27,6 +27,31 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/ ## ❗ BREAKING ❗ +### Rename traffic shaping deduplication options ([PR #XXXX](https://github.com/apollographql/router/pull/XXXX)) + +In the traffic shaping module: + - `variables_deduplication` configuration option is renamed to `deduplicate_variables`. + - `query_deduplication` configuration option is renamed to `deduplicate_query`. + +```diff +- traffic_shaping: +- variables_deduplication: true # Enable the variables deduplication optimization +- all: +- query_deduplication: true # Enable query deduplication for all subgraphs. +- subgraphs: +- products: +- query_deduplication: false # Disable query deduplication for products. ++ traffic_shaping: ++ deduplicate_variables: true # Enable the variables deduplication optimization ++ all: ++ deduplicate_query: true # Enable query deduplication for all subgraphs. ++ subgraphs: ++ products: ++ deduplicate_query: false # Disable query deduplication for products. +``` + +By [@garypen](https://github.com/garypen) + ### Put `query_plan_options` in private and wrap `QueryPlanContent` in an opaque type ([PR #1486](https://github.com/apollographql/router/pull/1486)) `QueryPlanOptions::query_plan_options` is no longer available in public. 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 ac22afd039..8ba09b3ca6 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,6 +1,5 @@ --- source: apollo-router/src/configuration/mod.rs -assertion_line: 909 expression: "&schema" --- { @@ -1884,6 +1883,11 @@ expression: "&schema" ], "nullable": true }, + "deduplicate_query": { + "description": "Enable query deduplication", + "type": "boolean", + "nullable": true + }, "global_rate_limit": { "description": "Enable global rate limiting", "type": "object", @@ -1906,11 +1910,6 @@ expression: "&schema" "additionalProperties": false, "nullable": true }, - "query_deduplication": { - "description": "Enable query deduplication", - "type": "boolean", - "nullable": true - }, "timeout": { "description": "Enable timeout for incoming requests", "default": null, @@ -1920,6 +1919,11 @@ expression: "&schema" "additionalProperties": false, "nullable": true }, + "deduplicate_variables": { + "description": "Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87)", + "type": "boolean", + "nullable": true + }, "router": { "description": "Applied at the router level", "type": "object", @@ -1971,6 +1975,11 @@ expression: "&schema" ], "nullable": true }, + "deduplicate_query": { + "description": "Enable query deduplication", + "type": "boolean", + "nullable": true + }, "global_rate_limit": { "description": "Enable global rate limiting", "type": "object", @@ -1993,11 +2002,6 @@ expression: "&schema" "additionalProperties": false, "nullable": true }, - "query_deduplication": { - "description": "Enable query deduplication", - "type": "boolean", - "nullable": true - }, "timeout": { "description": "Enable timeout for incoming requests", "default": null, @@ -2006,11 +2010,6 @@ expression: "&schema" }, "additionalProperties": false } - }, - "variables_deduplication": { - "description": "Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87)", - "type": "boolean", - "nullable": true } }, "additionalProperties": false diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index 5c9f236eb9..7514e875de 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -53,7 +53,7 @@ trait Merge { #[serde(deny_unknown_fields)] struct Shaping { /// Enable query deduplication - query_deduplication: Option, + deduplicate_query: Option, /// Enable compression for subgraphs (available compressions are deflate, br, gzip) compression: Option, /// Enable global rate limiting @@ -69,7 +69,7 @@ impl Merge for Shaping { match fallback { None => self.clone(), Some(fallback) => Shaping { - query_deduplication: self.query_deduplication.or(fallback.query_deduplication), + deduplicate_query: self.deduplicate_query.or(fallback.deduplicate_query), compression: self.compression.or(fallback.compression), timeout: self.timeout.or(fallback.timeout), global_rate_limit: self @@ -106,7 +106,7 @@ struct Config { /// Applied on specific subgraphs subgraphs: HashMap, /// Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87) - variables_deduplication: Option, + deduplicate_variables: Option, } #[derive(PartialEq, Debug, Clone, Deserialize, JsonSchema)] @@ -205,7 +205,7 @@ impl Plugin for TrafficShaping { .clone() }); ServiceBuilder::new() - .option_layer(config.query_deduplication.unwrap_or_default().then(|| { + .option_layer(config.deduplicate_query.unwrap_or_default().then(|| { // Buffer is required because dedup layer requires a clone service. ServiceBuilder::new() .layer(QueryDeduplicationLayer::default()) @@ -237,10 +237,10 @@ impl Plugin for TrafficShaping { &self, service: query_planner::BoxService, ) -> query_planner::BoxService { - if matches!(self.config.variables_deduplication, Some(true)) { + if matches!(self.config.deduplicate_variables, Some(true)) { service .map_request(|mut req: QueryPlannerRequest| { - req.query_plan_options.enable_variable_deduplication = true; + req.query_plan_options.enable_deduplicate_variables = true; req }) .boxed() @@ -381,7 +381,7 @@ mod test { async fn it_returns_valid_response_for_deduplicated_variables() { let config = serde_yaml::from_str::( r#" - variables_deduplication: true + deduplicate_variables: true "#, ) .unwrap(); @@ -436,10 +436,10 @@ mod test { let config = serde_yaml::from_str::( r#" all: - query_deduplication: true + deduplicate_query: true subgraphs: products: - query_deduplication: false + deduplicate_query: false "#, ) .unwrap(); diff --git a/apollo-router/src/query_planner/mod.rs b/apollo-router/src/query_planner/mod.rs index 0bb9196ecb..e7908a8c67 100644 --- a/apollo-router/src/query_planner/mod.rs +++ b/apollo-router/src/query_planner/mod.rs @@ -33,7 +33,7 @@ mod selection; #[derive(Clone, Eq, Hash, PartialEq, Debug, Default)] pub(crate) struct QueryPlanOptions { /// Enable the variable deduplication optimization on the QueryPlan - pub(crate) enable_variable_deduplication: bool, + pub(crate) enable_deduplicate_variables: bool, } /// A planner key. @@ -811,7 +811,7 @@ pub(crate) mod fetch { current_dir: &Path, request: &Arc>, schema: &Schema, - enable_variable_deduplication: bool, + enable_deduplicate_variables: bool, ) -> Option { let body = request.body(); if !requires.is_empty() { @@ -824,7 +824,7 @@ pub(crate) mod fetch { })); let mut paths: HashMap = HashMap::new(); - let (paths, representations) = if enable_variable_deduplication { + let (paths, representations) = if enable_deduplicate_variables { let mut values: IndexSet = IndexSet::new(); data.select_values_and_paths(current_dir, |path, value| { if let Value::Object(content) = value { @@ -924,7 +924,7 @@ pub(crate) mod fetch { // Needs the original request here parameters.originating_request, parameters.schema, - parameters.options.enable_variable_deduplication, + parameters.options.enable_deduplicate_variables, ) .await { diff --git a/apollo-router/src/query_planner/snapshots/apollo_router_core__query_planner__bridge_query_planner__tests__plan.snap b/apollo-router/src/query_planner/snapshots/apollo_router_core__query_planner__bridge_query_planner__tests__plan.snap deleted file mode 100644 index f5765d335a..0000000000 --- a/apollo-router/src/query_planner/snapshots/apollo_router_core__query_planner__bridge_query_planner__tests__plan.snap +++ /dev/null @@ -1,20 +0,0 @@ ---- -source: apollo-router-core/src/query_planner/bridge_query_planner.rs -assertion_line: 128 -expression: result ---- -QueryPlan { - root: Fetch( - FetchNode { - service_name: "accounts", - requires: [], - variable_usages: [], - operation: "{me{name{first last}}}", - operation_name: None, - operation_kind: Query, - }, - ), - options: QueryPlanOptions { - enable_variable_deduplication: false, - }, -} diff --git a/docs/source/configuration/traffic-shaping.mdx b/docs/source/configuration/traffic-shaping.mdx index 2a2216d2b8..c01b7b6168 100644 --- a/docs/source/configuration/traffic-shaping.mdx +++ b/docs/source/configuration/traffic-shaping.mdx @@ -21,18 +21,18 @@ To enable traffic shaping, add the `traffic_shaping` plugin to your [YAML config ```yaml title="router.yaml" traffic_shaping: - variables_deduplication: true # Enable the variable deduplication optimization. + deduplicate_variables: true # Enable the variable deduplication optimization. router: # Rules applied to requests from clients to the router global_rate_limit: # Accept a maximum of 10 requests per 5 secs. Excess requests must be rejected. capacity: 10 interval: 5s # Must not be greater than 18_446_744_073_709_551_615 milliseconds and not less than 0 milliseconds timeout: 50s # If a request to the router takes more than 50secs then cancel the request (30 sec by default) all: - query_deduplication: true # Enable query deduplication for all subgraphs. + deduplicate_query: true # Enable query deduplication for all subgraphs. compression: br # Enable brotli compression for all subgraphs. subgraphs: # Rules applied to requests from the router to individual subgraphs products: - query_deduplication: false # Disable query for the products subgraph. + deduplicate_query: false # Disable query deduplication for the products subgraph. compression: gzip # Enable gzip compression only for the products subgraph. global_rate_limit: # Accept a maximum of 10 requests per 5 secs from the router. Excess requests must be rejected. capacity: 10 From 5df668e771949b1003a3e6ca6303e45e07c2c907 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Thu, 18 Aug 2022 16:14:25 +0100 Subject: [PATCH 2/2] add PR# to changelog sigh... --- NEXT_CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index b3bfaf5a1d..d720bb132e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -27,7 +27,7 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/ ## ❗ BREAKING ❗ -### Rename traffic shaping deduplication options ([PR #XXXX](https://github.com/apollographql/router/pull/XXXX)) +### Rename traffic shaping deduplication options ([PR #1540](https://github.com/apollographql/router/pull/1540)) In the traffic shaping module: - `variables_deduplication` configuration option is renamed to `deduplicate_variables`.