From cfb80016e8786e87dbc69ac0099b043a44a85859 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 20 Jan 2023 12:52:56 +0100 Subject: [PATCH 1/3] always use variable deduplication this is an easy optimization that should be more largely used --- apollo-router/src/configuration/mod.rs | 7 --- ...nfiguration__tests__schema_generation.snap | 2 +- apollo-router/src/plugin/test/mock/canned.rs | 14 ----- .../src/plugins/expose_query_plan.rs | 8 +-- .../src/plugins/include_subgraph_errors.rs | 8 +-- .../src/plugins/traffic_shaping/mod.rs | 12 +---- .../src/query_planner/bridge_query_planner.rs | 10 ---- .../query_planner/caching_query_planner.rs | 2 - apollo-router/src/query_planner/execution.rs | 6 --- apollo-router/src/query_planner/fetch.rs | 52 ++++++------------- apollo-router/src/query_planner/plan.rs | 8 --- apollo-router/src/query_planner/tests.rs | 7 --- .../apollo_reports__client_name.snap | 40 -------------- .../apollo_reports__client_version.snap | 40 -------------- .../apollo_reports__condition_else.snap | 40 -------------- .../apollo_reports__condition_if.snap | 40 -------------- .../snapshots/apollo_reports__non_defer.snap | 40 -------------- .../apollo_reports__send_header.snap | 40 -------------- .../apollo_reports__send_variable_value.snap | 40 -------------- .../snapshots/apollo_reports__trace_id.snap | 40 -------------- 20 files changed, 27 insertions(+), 429 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 7c99db3658..9caf53dd9a 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -293,13 +293,6 @@ impl Configuration { plugins } - pub(crate) fn plugin_configuration(&self, plugin_name: &str) -> Option { - self.plugins() - .iter() - .find(|(name, _)| name == plugin_name) - .map(|(_, value)| value.clone()) - } - // checks that we can reload configuration from the current one to the new one pub(crate) fn is_compatible(&self, new: &Configuration) -> Result<(), &'static str> { if self.apollo_plugins.plugins.get(TELEMETRY_KEY) 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 0a51b77d05..1a08864544 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 @@ -3223,7 +3223,7 @@ expression: "&schema" "nullable": true }, "deduplicate_variables": { - "description": "Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87)", + "description": "DEPRECATED, now always enabled: Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87)", "type": "boolean", "nullable": true }, diff --git a/apollo-router/src/plugin/test/mock/canned.rs b/apollo-router/src/plugin/test/mock/canned.rs index 66dac25a17..5230ed3cab 100644 --- a/apollo-router/src/plugin/test/mock/canned.rs +++ b/apollo-router/src/plugin/test/mock/canned.rs @@ -21,10 +21,6 @@ pub(crate) fn accounts_subgraph() -> MockSubgraph { "__typename": "User", "id": "2" }, - { - "__typename": "User", - "id": "1" - } ] } }}, @@ -37,9 +33,6 @@ pub(crate) fn accounts_subgraph() -> MockSubgraph { { "name": "Alan Turing" }, - { - "name": "Ada Lovelace" - } ] } }} @@ -158,10 +151,6 @@ pub(crate) fn products_subgraph() -> MockSubgraph { "__typename": "Product", "upc": "1" }, - { - "__typename": "Product", - "upc": "1" - }, { "__typename": "Product", "upc": "2" @@ -172,9 +161,6 @@ pub(crate) fn products_subgraph() -> MockSubgraph { json!{{ "data": { "_entities": [ - { - "name": "Table" - }, { "name": "Table" }, diff --git a/apollo-router/src/plugins/expose_query_plan.rs b/apollo-router/src/plugins/expose_query_plan.rs index 25a33c214d..8a64790b11 100644 --- a/apollo-router/src/plugins/expose_query_plan.rs +++ b/apollo-router/src/plugins/expose_query_plan.rs @@ -158,8 +158,8 @@ mod tests { let account_mocks = vec![ ( - r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"},{"__typename":"User","id":"1"}]}}"#, - r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"},{"name":"Ada Lovelace"}]}}"# + r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"}]}}"#, + r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"}]}}"# ) ].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect(); let account_service = MockSubgraph::new(account_mocks); @@ -178,8 +178,8 @@ mod tests { r#"{"data":{"topProducts":[{"__typename":"Product","upc":"1","name":"Table"},{"__typename":"Product","upc":"2","name":"Couch"}]}}"# ), ( - r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#, - r#"{"data":{"_entities":[{"name":"Table"},{"name":"Table"},{"name":"Couch"}]}}"# + r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#, + r#"{"data":{"_entities":[{"name":"Table"},{"name":"Couch"}]}}"# ) ].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect(); diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs index 62033d18a4..80c792ffe3 100644 --- a/apollo-router/src/plugins/include_subgraph_errors.rs +++ b/apollo-router/src/plugins/include_subgraph_errors.rs @@ -159,8 +159,8 @@ mod test { let account_mocks = vec![ ( - r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"},{"__typename":"User","id":"1"}]}}"#, - r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"},{"name":"Ada Lovelace"}]}}"# + r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"}]}}"#, + r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"}]}}"# ) ].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect(); let account_service = MockSubgraph::new(account_mocks); @@ -179,8 +179,8 @@ mod test { r#"{"data":{"topProducts":[{"__typename":"Product","upc":"1","name":"Table"},{"__typename":"Product","upc":"2","name":"Couch"}]}}"# ), ( - r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#, - r#"{"data":{"_entities":[{"name":"Table"},{"name":"Table"},{"name":"Couch"}]}}"# + r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#, + r#"{"data":{"_entities":[{"name":"Table"},{"name":"Couch"}]}}"# ) ].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect(); diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index 94ba1ebe6e..a7da83ef9c 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -45,7 +45,6 @@ use crate::services::subgraph; use crate::services::subgraph_service::Compression; use crate::services::supergraph; use crate::services::SubgraphRequest; -use crate::Configuration; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); pub(crate) const APOLLO_TRAFFIC_SHAPING: &str = "apollo.traffic_shaping"; @@ -161,7 +160,7 @@ pub(crate) struct Config { #[serde(default)] /// Applied on specific subgraphs subgraphs: HashMap, - /// Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87) + /// DEPRECATED, now always enabled: Enable variable deduplication optimization when sending requests to subgraphs (https://github.com/apollographql/router/issues/87) deduplicate_variables: Option, } @@ -368,15 +367,6 @@ impl TrafficShaping { } } -impl TrafficShaping { - pub(crate) fn get_configuration_deduplicate_variables(configuration: &Configuration) -> bool { - configuration - .plugin_configuration(APOLLO_TRAFFIC_SHAPING) - .map(|conf| conf.get("deduplicate_variables") == Some(&serde_json::Value::Bool(true))) - .unwrap_or_default() - } -} - register_plugin!("apollo", "traffic_shaping", TrafficShaping); #[cfg(test)] diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 504c25ec17..849077eb0a 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -17,11 +17,9 @@ use tracing::Instrument; use super::PlanNode; use super::QueryKey; -use super::QueryPlanOptions; use crate::error::QueryPlannerError; use crate::graphql; use crate::introspection::Introspection; -use crate::plugins::traffic_shaping::TrafficShaping; use crate::services::QueryPlannerContent; use crate::services::QueryPlannerRequest; use crate::services::QueryPlannerResponse; @@ -41,7 +39,6 @@ pub(crate) struct BridgeQueryPlanner { schema: Arc, introspection: Option>, configuration: Arc, - deduplicate_variables: bool, } impl BridgeQueryPlanner { @@ -50,9 +47,6 @@ impl BridgeQueryPlanner { introspection: Option>, configuration: Arc, ) -> Result { - // FIXME: The variables deduplication parameter lives in the traffic_shaping section of the config - let deduplicate_variables = - TrafficShaping::get_configuration_deduplicate_variables(&configuration); Ok(Self { planner: Arc::new( Planner::new( @@ -68,7 +62,6 @@ impl BridgeQueryPlanner { schema, introspection, configuration, - deduplicate_variables, }) } @@ -134,9 +127,6 @@ impl BridgeQueryPlanner { root: node, formatted_query_plan, query: Arc::new(selections), - options: QueryPlanOptions { - enable_deduplicate_variables: self.deduplicate_variables, - }, }), }) } diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index ce1a7a2205..afeebbf0fe 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -288,7 +288,6 @@ mod tests { use super::*; use crate::error::PlanErrors; use crate::query_planner::QueryPlan; - use crate::query_planner::QueryPlanOptions; use crate::spec::Query; mock! { @@ -384,7 +383,6 @@ mod tests { let query_plan: QueryPlan = QueryPlan { formatted_query_plan: Default::default(), root: serde_json::from_str(test_query_plan!()).unwrap(), - options: QueryPlanOptions::default(), usage_reporting: UsageReporting { stats_report_key: "this is a test report key".to_string(), referenced_fields_by_type: Default::default(), diff --git a/apollo-router/src/query_planner/execution.rs b/apollo-router/src/query_planner/execution.rs index 263926b87b..d273a033ef 100644 --- a/apollo-router/src/query_planner/execution.rs +++ b/apollo-router/src/query_planner/execution.rs @@ -11,7 +11,6 @@ use super::log; use super::DeferredNode; use super::PlanNode; use super::QueryPlan; -use super::QueryPlanOptions; use crate::error::Error; use crate::graphql::Request; use crate::graphql::Response; @@ -59,7 +58,6 @@ impl QueryPlan { supergraph_request, deferred_fetches: &deferred_fetches, query: &self.query, - options: &self.options, }, &root, &Value::default(), @@ -87,7 +85,6 @@ pub(crate) struct ExecutionParameters<'a> { pub(crate) supergraph_request: &'a Arc>, pub(crate) deferred_fetches: &'a HashMap)>>, pub(crate) query: &'a Arc, - pub(crate) options: &'a QueryPlanOptions, } impl PlanNode { @@ -247,7 +244,6 @@ impl PlanNode { schema: parameters.schema, supergraph_request: parameters.supergraph_request, deferred_fetches: &deferred_fetches, - options: parameters.options, query: parameters.query, }, current_dir, @@ -391,7 +387,6 @@ impl DeferredNode { let orig = parameters.supergraph_request.clone(); let sf = parameters.service_factory.clone(); let ctx = parameters.context.clone(); - let opt = parameters.options.clone(); let query = parameters.query.clone(); let mut primary_receiver = primary_sender.subscribe(); let mut value = parent_value.clone(); @@ -429,7 +424,6 @@ impl DeferredNode { supergraph_request: &orig, deferred_fetches: &deferred_fetches, query: &query, - options: &opt, }, &Path::default(), &value, diff --git a/apollo-router/src/query_planner/fetch.rs b/apollo-router/src/query_planner/fetch.rs index 922d1b6125..efced4864a 100644 --- a/apollo-router/src/query_planner/fetch.rs +++ b/apollo-router/src/query_planner/fetch.rs @@ -99,7 +99,6 @@ impl Variables { current_dir: &Path, request: &Arc>, schema: &Schema, - enable_deduplicate_variables: bool, ) -> Option { let body = request.body(); if !requires.is_empty() { @@ -112,46 +111,30 @@ impl Variables { })); let mut paths: HashMap = HashMap::new(); - 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 { - if let Ok(Some(value)) = select_object(content, requires, schema) { - match values.get_index_of(&value) { - Some(index) => { - paths.insert(path.clone(), index); - } - None => { - paths.insert(path.clone(), values.len()); - values.insert(value); - } + let mut values: IndexSet = IndexSet::new(); + + data.select_values_and_paths(current_dir, |path, value| { + if let Value::Object(content) = value { + if let Ok(Some(value)) = select_object(content, requires, schema) { + match values.get_index_of(&value) { + Some(index) => { + paths.insert(path.clone(), index); + } + None => { + paths.insert(path.clone(), values.len()); + values.insert(value); } } } - }); - - if values.is_empty() { - return None; } + }); - (paths, Value::Array(Vec::from_iter(values))) - } else { - let mut values: Vec = Vec::new(); - data.select_values_and_paths(current_dir, |path, value| { - if let Value::Object(content) = value { - if let Ok(Some(value)) = select_object(content, requires, schema) { - paths.insert(path.clone(), values.len()); - values.push(value); - } - } - }); + if values.is_empty() { + return None; + } - if values.is_empty() { - return None; - } + let representations = Value::Array(Vec::from_iter(values)); - (paths, Value::Array(Vec::from_iter(values))) - }; variables.insert("representations", representations); Some(Variables { variables, paths }) @@ -209,7 +192,6 @@ impl FetchNode { // Needs the original request here parameters.supergraph_request, parameters.schema, - parameters.options.enable_deduplicate_variables, ) .await { diff --git a/apollo-router/src/query_planner/plan.rs b/apollo-router/src/query_planner/plan.rs index fad6c9edf4..3671b0e631 100644 --- a/apollo-router/src/query_planner/plan.rs +++ b/apollo-router/src/query_planner/plan.rs @@ -17,12 +17,6 @@ use crate::spec::query::SubSelection; use crate::spec::Query; use crate::spec::Schema; -/// Query planning options. -#[derive(Clone, Eq, Hash, PartialEq, Debug, Default, Serialize, Deserialize)] -pub(crate) struct QueryPlanOptions { - /// Enable the variable deduplication optimization on the QueryPlan - pub(crate) enable_deduplicate_variables: bool, -} /// A planner key. /// /// This type consists of a query string and an optional operation string @@ -36,7 +30,6 @@ pub struct QueryPlan { /// String representation of the query plan (not a json representation) pub(crate) formatted_query_plan: Option, pub(crate) query: Arc, - pub(crate) options: QueryPlanOptions, } /// This default impl is useful for test users @@ -56,7 +49,6 @@ impl QueryPlan { root: root.unwrap_or_else(|| PlanNode::Sequence { nodes: Vec::new() }), formatted_query_plan: Default::default(), query: Arc::new(Query::default()), - options: QueryPlanOptions::default(), } } } diff --git a/apollo-router/src/query_planner/tests.rs b/apollo-router/src/query_planner/tests.rs index 8c690243eb..7669d91f44 100644 --- a/apollo-router/src/query_planner/tests.rs +++ b/apollo-router/src/query_planner/tests.rs @@ -15,7 +15,6 @@ use super::OperationKind; use super::PlanNode; use super::Primary; use super::QueryPlan; -use super::QueryPlanOptions; use crate::json_ext::Path; use crate::json_ext::PathElement; use crate::plugin; @@ -74,7 +73,6 @@ async fn mock_subgraph_service_withf_panics_should_be_reported_as_service_closed let query_plan: QueryPlan = QueryPlan { root: serde_json::from_str(test_query_plan!()).unwrap(), formatted_query_plan: Default::default(), - options: QueryPlanOptions::default(), query: Arc::new(Query::default()), usage_reporting: UsageReporting { stats_report_key: "this is a test report key".to_string(), @@ -129,7 +127,6 @@ async fn fetch_includes_operation_name() { referenced_fields_by_type: Default::default(), }, query: Arc::new(Query::default()), - options: QueryPlanOptions::default(), }; let succeeded: Arc = Default::default(); @@ -184,7 +181,6 @@ async fn fetch_makes_post_requests() { referenced_fields_by_type: Default::default(), }, query: Arc::new(Query::default()), - options: QueryPlanOptions::default(), }; let succeeded: Arc = Default::default(); @@ -298,7 +294,6 @@ async fn defer() { referenced_fields_by_type: Default::default(), }, query: Arc::new(Query::default()), - options: QueryPlanOptions::default(), }; let mut mock_x_service = plugin::test::MockSubgraphService::new(); @@ -410,7 +405,6 @@ async fn defer_if_condition() { ) .unwrap(), ), - options: QueryPlanOptions::default(), formatted_query_plan: None, }; @@ -575,7 +569,6 @@ async fn dependent_mutations() { referenced_fields_by_type: Default::default(), }, query: Arc::new(Query::default()), - options: QueryPlanOptions::default(), }; let mut mock_a_service = plugin::test::MockSubgraphService::new(); diff --git a/apollo-router/tests/snapshots/apollo_reports__client_name.snap b/apollo-router/tests/snapshots/apollo_reports__client_name.snap index 689f4bf411..453bdb33b9 100644 --- a/apollo-router/tests/snapshots/apollo_reports__client_name.snap +++ b/apollo-router/tests/snapshots/apollo_reports__client_name.snap @@ -506,46 +506,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__client_version.snap b/apollo-router/tests/snapshots/apollo_reports__client_version.snap index 88f6018138..2ccc319a98 100644 --- a/apollo-router/tests/snapshots/apollo_reports__client_version.snap +++ b/apollo-router/tests/snapshots/apollo_reports__client_version.snap @@ -506,46 +506,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__condition_else.snap b/apollo-router/tests/snapshots/apollo_reports__condition_else.snap index 546d9328db..950edbc3d4 100644 --- a/apollo-router/tests/snapshots/apollo_reports__condition_else.snap +++ b/apollo-router/tests/snapshots/apollo_reports__condition_else.snap @@ -512,46 +512,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__condition_if.snap b/apollo-router/tests/snapshots/apollo_reports__condition_if.snap index 6c35f27d4e..7b92d854a1 100644 --- a/apollo-router/tests/snapshots/apollo_reports__condition_if.snap +++ b/apollo-router/tests/snapshots/apollo_reports__condition_if.snap @@ -524,46 +524,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__non_defer.snap b/apollo-router/tests/snapshots/apollo_reports__non_defer.snap index f9473b4250..719b789080 100644 --- a/apollo-router/tests/snapshots/apollo_reports__non_defer.snap +++ b/apollo-router/tests/snapshots/apollo_reports__non_defer.snap @@ -506,46 +506,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__send_header.snap b/apollo-router/tests/snapshots/apollo_reports__send_header.snap index 4e76082eea..dc0071df94 100644 --- a/apollo-router/tests/snapshots/apollo_reports__send_header.snap +++ b/apollo-router/tests/snapshots/apollo_reports__send_header.snap @@ -509,46 +509,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__send_variable_value.snap b/apollo-router/tests/snapshots/apollo_reports__send_variable_value.snap index 2385725b5e..62be713610 100644 --- a/apollo-router/tests/snapshots/apollo_reports__send_variable_value.snap +++ b/apollo-router/tests/snapshots/apollo_reports__send_variable_value.snap @@ -508,46 +508,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ diff --git a/apollo-router/tests/snapshots/apollo_reports__trace_id.snap b/apollo-router/tests/snapshots/apollo_reports__trace_id.snap index f9473b4250..719b789080 100644 --- a/apollo-router/tests/snapshots/apollo_reports__trace_id.snap +++ b/apollo-router/tests/snapshots/apollo_reports__trace_id.snap @@ -506,46 +506,6 @@ traces_per_query: ResponseName: name id: Index: 1 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 2 - - original_field_name: "" - type: "" - parent_type: "" - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: - - original_field_name: "" - type: String - parent_type: User - cache_policy: ~ - start_time: "[start_time]" - end_time: "[end_time]" - error: [] - child: [] - id: - ResponseName: name - id: - Index: 3 id: ResponseName: _entities id: ~ From 6340db78903b0a057ce54690ee0a8b22c04e4cbb Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 20 Jan 2023 13:13:59 +0100 Subject: [PATCH 2/3] changelog and docs --- NEXT_CHANGELOG.md | 5 +++++ docs/source/configuration/traffic-shaping.mdx | 8 +------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 19804b40fd..4bccb5312c 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -129,6 +129,11 @@ Users can disable this mechanism by setting the environment variable `APOLLO_TEL By [@bryncooke](https://github.com/bryncooke) in https://github.com/apollographql/router/pull/2173, https://github.com/apollographql/router/issues/2398, https://github.com/apollographql/router/pull/2413 +### Always deduplicate variables ([Issue #2387](https://github.com/apollographql/router/issues/2387)) + +Variable deduplication allows the router to reduce the number of entities that are requested from subgraphs if some of them are redundant, and as such reduce the size of subgraph responses. It has been available for a while but was not active by default. This is now always on. + +By [@Geal](https://github.com/geal) in https://github.com/apollographql/router/pull/2445 ## 🐛 Fixes diff --git a/docs/source/configuration/traffic-shaping.mdx b/docs/source/configuration/traffic-shaping.mdx index 4bd436dd6e..e12245e45c 100644 --- a/docs/source/configuration/traffic-shaping.mdx +++ b/docs/source/configuration/traffic-shaping.mdx @@ -11,7 +11,6 @@ To enable traffic shaping, add the `traffic_shaping` plugin to your [YAML config ```yaml title="router.yaml" traffic_shaping: - 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 @@ -133,12 +132,7 @@ traffic_shaping: When subgraphs are sent entity requests by the Router using the `_entities` field, it is often the case that the same entity (identified by a unique `@key` constraint) is requested multiple times within the execution of a single federated query. For example, an author's name might need to be fetched multiple times when accessing a list of a reviews for a product for which the author has written multiple reviews. -To reduce the size of subgraph requests and the amount of work they might perform, the list of entities sent can be deduplicated. This is activated with the `deduplicate_variables` option: - -```yaml title="router.yaml" -traffic_shaping: - deduplicate_variables: true # Enable the variable deduplication optimization. -``` +To reduce the size of subgraph requests and the amount of work they might perform, the list of entities sent can be deduplicated. This is always active. ### Query deduplication From 0e7aba28d0ec58ebefd14d51690d0313bccbd8f7 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 24 Jan 2023 09:37:20 +0100 Subject: [PATCH 3/3] fix benchmarks test --- apollo-router-benchmarks/src/shared.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/apollo-router-benchmarks/src/shared.rs b/apollo-router-benchmarks/src/shared.rs index 15f0f67ed1..616abb7c26 100644 --- a/apollo-router-benchmarks/src/shared.rs +++ b/apollo-router-benchmarks/src/shared.rs @@ -58,10 +58,6 @@ pub fn setup() -> TestHarness<'static> { { "__typename": "User", "id": "2" - }, - { - "__typename": "User", - "id": "1" } ] } @@ -74,9 +70,6 @@ pub fn setup() -> TestHarness<'static> { }, { "name": "Alan Turing" - }, - { - "name": "Ada Lovelace" } ] } @@ -177,10 +170,6 @@ pub fn setup() -> TestHarness<'static> { "__typename": "Product", "upc": "1" }, - { - "__typename": "Product", - "upc": "1" - }, { "__typename": "Product", "upc": "2" @@ -191,9 +180,6 @@ pub fn setup() -> TestHarness<'static> { json!{{ "data": { "_entities": [ - { - "name": "Table" - }, { "name": "Table" },