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

Variable deduplication in query planner #872

Merged
merged 17 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ In addition, the `activate` lifecycle hook is now not marked as deprecated, and

## 🚀 Features

### Add an experimental optimization to deduplicate variables in query planner [PR #872](https://github.com/apollographql/router/pull/872)
Get rid of duplicated variables in requests and responses of the query planner. This optimization is disabled by default, if you want to enable it you just need override your configuration:

```yaml title="router.yaml"
server:
experimental:
enable_variable_deduplication: true
```

### Add SpanKind and SpanStatusCode to follow the opentelemetry spec [PR #925](https://github.com/apollographql/router/pull/925)
Spans now contains [`otel.kind`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spankind) and [`otel.status_code`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status) attributes when needed to follow the opentelemtry spec .

Expand Down
4 changes: 2 additions & 2 deletions apollo-router-core/src/json_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ where
/// A GraphQL path element that is composes of strings or numbers.
/// e.g `/book/3/name`
#[doc(hidden)]
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Hash)]
#[serde(untagged)]
pub enum PathElement {
/// A path element that given an array will flatmap the content.
Expand Down Expand Up @@ -424,7 +424,7 @@ where
///
/// This can be composed of strings and numbers
#[doc(hidden)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Default, Hash)]
#[serde(transparent)]
pub struct Path(pub Vec<PathElement>);

Expand Down
2 changes: 1 addition & 1 deletion apollo-router-core/src/layers/forbid_http_get_mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ mod forbid_http_get_mutations_tests {

ExecutionRequest::fake_builder()
.originating_request(request)
.query_plan(Arc::new(QueryPlan { root }))
.query_plan(Arc::new(QueryPlan::new(root)))
.build()
}
}
2 changes: 1 addition & 1 deletion apollo-router-core/src/plugins/forbid_mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ mod forbid_http_get_mutations_tests {
.expect("expecting valid request");
ExecutionRequest::fake_builder()
.originating_request(request)
.query_plan(Arc::new(QueryPlan { root }))
.query_plan(Arc::new(QueryPlan::new(root)))
.build()
}
}
Expand Down
122 changes: 119 additions & 3 deletions apollo-router-core/src/plugins/include_subgraph_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ mod test {
use super::*;
use crate::plugin::utils::test::mock::subgraph::MockSubgraph;
use crate::{
DynPlugin, Object, PluggableRouterServiceBuilder, Response, ResponseBody, RouterRequest,
RouterResponse, Schema,
DynPlugin, Object, PluggableRouterServiceBuilder, QueryPlanOptions, Response, ResponseBody,
RouterRequest, RouterResponse, Schema,
};
use bytes::Bytes;
use serde_json::Value as jValue;
Expand Down Expand Up @@ -169,7 +169,6 @@ mod test {
);

let builder = PluggableRouterServiceBuilder::new(schema.clone());

let builder = builder
.with_dyn_plugin("experimental.include_subgraph_errors".to_string(), plugin)
.with_subgraph_service("accounts", account_service.clone())
Expand All @@ -181,6 +180,63 @@ mod test {
router
}

async fn build_mock_router_with_variable_dedup_optimization(
plugin: Box<dyn DynPlugin>,
) -> BoxCloneService<RouterRequest, RouterResponse, BoxError> {
let mut extensions = Object::new();
extensions.insert("test", Value::String(ByteString::from("value")));

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"}]}}"#,
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);

let review_mocks = vec![
(
r#"{"query":"query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{id product{__typename upc}author{__typename id}}}}}","operationName":"TopProducts__reviews__1","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#,
r#"{"data":{"_entities":[{"reviews":[{"id":"1","product":{"__typename":"Product","upc":"1"},"author":{"__typename":"User","id":"1"}},{"id":"4","product":{"__typename":"Product","upc":"1"},"author":{"__typename":"User","id":"2"}}]},{"reviews":[{"id":"2","product":{"__typename":"Product","upc":"2"},"author":{"__typename":"User","id":"1"}}]}]}}"#
)
].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect();
let review_service = MockSubgraph::new(review_mocks);

let product_mocks = vec![
(
r#"{"query":"query TopProducts__products__0($first:Int){topProducts(first:$first){__typename upc name}}","operationName":"TopProducts__products__0","variables":{"first":2}}"#,
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":"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();

let product_service = MockSubgraph::new(product_mocks).with_extensions(extensions);

let schema: Arc<Schema> = Arc::new(
include_str!("../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql")
.parse()
.unwrap(),
);

let builder = PluggableRouterServiceBuilder::new(schema.clone());

let builder = builder
.with_dyn_plugin("experimental.include_subgraph_errors".to_string(), plugin)
.with_subgraph_service("accounts", account_service.clone())
.with_subgraph_service("reviews", review_service.clone())
.with_subgraph_service("products", product_service.clone())
.with_query_plan_options(QueryPlanOptions {
enable_variable_deduplication: true,
});

let (router, _) = builder.build().await.expect("should build");

router
}

async fn get_redacting_plugin(config: &jValue) -> Box<dyn DynPlugin> {
// Build a redacting plugin
crate::plugins()
Expand All @@ -197,6 +253,9 @@ mod test {
let plugin = get_redacting_plugin(&serde_json::json!({ "all": false })).await;
let router = build_mock_router(plugin).await;
execute_router_test(VALID_QUERY, &*EXPECTED_RESPONSE, router).await;
let plugin = get_redacting_plugin(&serde_json::json!({ "all": false })).await;
let router = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(VALID_QUERY, &*EXPECTED_RESPONSE, router).await;
}

#[tokio::test]
Expand All @@ -205,6 +264,15 @@ mod test {
let plugin = get_redacting_plugin(&serde_json::json!({ "all": false })).await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*REDACTED_PRODUCT_RESPONSE, router).await;

let plugin = get_redacting_plugin(&serde_json::json!({ "all": false })).await;
let router_with_opti = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(
ERROR_PRODUCT_QUERY,
&*REDACTED_PRODUCT_RESPONSE,
router_with_opti,
)
.await;
}

#[tokio::test]
Expand All @@ -213,6 +281,15 @@ mod test {
let plugin = get_redacting_plugin(&serde_json::json!({})).await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*REDACTED_PRODUCT_RESPONSE, router).await;

let plugin = get_redacting_plugin(&serde_json::json!({})).await;
let router_with_opti = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(
ERROR_PRODUCT_QUERY,
&*REDACTED_PRODUCT_RESPONSE,
router_with_opti,
)
.await;
}

#[tokio::test]
Expand All @@ -221,6 +298,9 @@ mod test {
let plugin = get_redacting_plugin(&serde_json::json!({ "all": true })).await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
let plugin = get_redacting_plugin(&serde_json::json!({ "all": true })).await;
let router = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
}

#[tokio::test]
Expand All @@ -230,6 +310,10 @@ mod test {
get_redacting_plugin(&serde_json::json!({ "subgraphs": {"products": true }})).await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
let plugin =
get_redacting_plugin(&serde_json::json!({ "subgraphs": {"products": true }})).await;
let router = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
}

#[tokio::test]
Expand All @@ -239,6 +323,15 @@ mod test {
get_redacting_plugin(&serde_json::json!({ "subgraphs": {"reviews": true }})).await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*REDACTED_PRODUCT_RESPONSE, router).await;
let plugin =
get_redacting_plugin(&serde_json::json!({ "subgraphs": {"reviews": true }})).await;
let router_with_opti = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(
ERROR_PRODUCT_QUERY,
&*REDACTED_PRODUCT_RESPONSE,
router_with_opti,
)
.await;
}

#[tokio::test]
Expand All @@ -250,6 +343,12 @@ mod test {
.await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
let plugin = get_redacting_plugin(
&serde_json::json!({ "all": true, "subgraphs": {"reviews": false }}),
)
.await;
let router = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*UNREDACTED_PRODUCT_RESPONSE, router).await;
}

#[tokio::test]
Expand All @@ -261,6 +360,17 @@ mod test {
.await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_PRODUCT_QUERY, &*REDACTED_PRODUCT_RESPONSE, router).await;
let plugin = get_redacting_plugin(
&serde_json::json!({ "all": true, "subgraphs": {"products": false }}),
)
.await;
let router_with_opti = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(
ERROR_PRODUCT_QUERY,
&*REDACTED_PRODUCT_RESPONSE,
router_with_opti,
)
.await;
}

#[tokio::test]
Expand All @@ -283,5 +393,11 @@ mod test {
.await;
let router = build_mock_router(plugin).await;
execute_router_test(ERROR_ACCOUNT_QUERY, &*REDACTED_ACCOUNT_RESPONSE, router).await;
let plugin = get_redacting_plugin(
&serde_json::json!({ "all": true, "subgraphs": {"accounts": false }}),
)
.await;
let router = build_mock_router_with_variable_dedup_optimization(plugin).await;
execute_router_test(ERROR_ACCOUNT_QUERY, &*REDACTED_ACCOUNT_RESPONSE, router).await;
}
}
6 changes: 4 additions & 2 deletions apollo-router-core/src/query_planner/bridge_query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl QueryPlanner for BridgeQueryPlanner {
&self,
query: String,
operation: Option<String>,
_options: QueryPlanOptions,
options: QueryPlanOptions,
) -> Result<Arc<QueryPlan>, QueryPlannerError> {
let planner_result = self
.planner
Expand All @@ -81,7 +81,9 @@ impl QueryPlanner for BridgeQueryPlanner {
.map_err(QueryPlannerError::from)?;

match planner_result {
PlannerResult::QueryPlan { node: Some(node) } => Ok(Arc::new(QueryPlan { root: node })),
PlannerResult::QueryPlan { node: Some(node) } => {
Ok(Arc::new(QueryPlan::new(node).with_options(options)))
}
PlannerResult::QueryPlan { node: None } => {
failfast_debug!("empty query plan");
Err(QueryPlannerError::EmptyPlan)
Expand Down
10 changes: 9 additions & 1 deletion apollo-router-core/src/query_planner/caching_query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type PlanResult = Result<Arc<QueryPlan>, QueryPlannerError>;
pub struct CachingQueryPlanner<T: QueryPlanner> {
cm: Arc<CachingMap<QueryKey, Arc<QueryPlan>>>,
phantom: PhantomData<T>,
options: QueryPlanOptions,
}

/// A resolver for cache misses
Expand All @@ -30,9 +31,16 @@ impl<T: QueryPlanner + 'static> CachingQueryPlanner<T> {
Self {
cm,
phantom: PhantomData,
options: QueryPlanOptions::default(),
}
}

/// Change options of the QueryPlan
pub fn with_options(mut self, options: QueryPlanOptions) -> CachingQueryPlanner<T> {
self.options = options;
self
}

pub async fn get_hot_keys(&self) -> Vec<QueryKey> {
self.cm.get_hot_keys().await
}
Expand Down Expand Up @@ -86,7 +94,7 @@ where
.clone()
.expect("presence of a query has been checked by the RouterService before; qed"),
body.operation_name.to_owned(),
QueryPlanOptions::default(),
self.options.clone(),
);
let cm = self.cm.clone();
Box::pin(async move {
Expand Down