Skip to content

Commit

Permalink
Merge branch 'dev' into geal/fix-query-hashing
Browse files Browse the repository at this point in the history
  • Loading branch information
@Geal
Geal committed Nov 5, 2024
2 parents 8302c45 + 36bdb5e commit 71209e3
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 41 deletions.
8 changes: 8 additions & 0 deletions .changesets/maint_geal_query_planner_cache_key_improvements.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### Query planner cache key improvements ([Issue #5160](https://github.com/apollographql/router/issues/5160))

> [!IMPORTANT]
> If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), this release changes the hashing algorithm used for the cache keys. On account of this, you should anticipate additional cache regeneration cost when updating between these versions while the new hashing algorithm comes into service.
This brings several performance improvements to the query plan cache key generation. In particular, it changes the distributed cache's key format, adding prefixes to the different key segments, to help in debugging.

By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/6206
16 changes: 16 additions & 0 deletions apollo-router/src/configuration/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,22 @@ impl Configuration {
type_conditioned_fetching: self.experimental_type_conditioned_fetching,
}
}

pub(crate) fn rust_query_planner_config(
&self,
) -> apollo_federation::query_plan::query_planner::QueryPlannerConfig {
apollo_federation::query_plan::query_planner::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,
},
type_conditioned_fetching: self.experimental_type_conditioned_fetching,
debug: Default::default(),
}
}
}

impl Default for Configuration {
Expand Down
99 changes: 64 additions & 35 deletions apollo-router/src/query_planner/caching_query_planner.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::hash::Hash;
use std::hash::Hasher;
use std::ops::Deref;
use std::sync::Arc;
use std::task;
Expand All @@ -14,7 +15,6 @@ use router_bridge::planner::PlanOptions;
use router_bridge::planner::Planner;
use router_bridge::planner::QueryPlannerConfig;
use router_bridge::planner::UsageReporting;
use serde::Serialize;
use sha2::Digest;
use sha2::Sha256;
use tower::BoxError;
Expand Down Expand Up @@ -57,11 +57,11 @@ pub(crate) type InMemoryCachePlanner =
InMemoryCache<CachingQueryKey, Result<QueryPlannerContent, Arc<QueryPlannerError>>>;
pub(crate) const APOLLO_OPERATION_ID: &str = "apollo_operation_id";

#[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize)]
#[derive(Debug, Clone, Hash)]
pub(crate) enum ConfigMode {
//FIXME: add the Rust planner structure once it is hashable and serializable,
// for now use the JS config as it expected to be identical to the Rust one
Rust(Arc<QueryPlannerConfig>),
Rust(Arc<apollo_federation::query_plan::query_planner::QueryPlannerConfig>),
Both(Arc<QueryPlannerConfig>),
BothBestEffort(Arc<QueryPlannerConfig>),
Js(Arc<QueryPlannerConfig>),
Expand All @@ -80,7 +80,7 @@ pub(crate) struct CachingQueryPlanner<T: Clone> {
subgraph_schemas: Arc<HashMap<String, Arc<Valid<apollo_compiler::Schema>>>>,
plugins: Arc<Plugins>,
enable_authorization_directives: bool,
config_mode: ConfigMode,
config_mode_hash: Arc<QueryHash>,
}

fn init_query_plan_from_redis(
Expand Down Expand Up @@ -125,28 +125,42 @@ where
let enable_authorization_directives =
AuthorizationPlugin::enable_directives(configuration, &schema).unwrap_or(false);

let config_mode = match configuration.experimental_query_planner_mode {
let mut hasher = StructHasher::new();
match configuration.experimental_query_planner_mode {
crate::configuration::QueryPlannerMode::New => {
ConfigMode::Rust(Arc::new(configuration.js_query_planner_config()))
"PLANNER-NEW".hash(&mut hasher);
ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config()))
.hash(&mut hasher);
}
crate::configuration::QueryPlannerMode::Legacy => {
ConfigMode::Js(Arc::new(configuration.js_query_planner_config()))
"PLANNER-LEGACY".hash(&mut hasher);
ConfigMode::Js(Arc::new(configuration.js_query_planner_config())).hash(&mut hasher);
}
crate::configuration::QueryPlannerMode::Both => {
"PLANNER-BOTH".hash(&mut hasher);
ConfigMode::Both(Arc::new(configuration.js_query_planner_config()))
.hash(&mut hasher);
ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config()))
.hash(&mut hasher);
}
crate::configuration::QueryPlannerMode::BothBestEffort => {
"PLANNER-BOTH-BEST-EFFORT".hash(&mut hasher);
ConfigMode::BothBestEffort(Arc::new(configuration.js_query_planner_config()))
.hash(&mut hasher);
ConfigMode::Rust(Arc::new(configuration.rust_query_planner_config()))
.hash(&mut hasher);
}
};
let config_mode_hash = Arc::new(QueryHash(hasher.finalize()));

Ok(Self {
cache,
delegate,
schema,
subgraph_schemas,
plugins: Arc::new(plugins),
enable_authorization_directives,
config_mode,
config_mode_hash,
})
}

Expand Down Expand Up @@ -204,7 +218,7 @@ where
hash: Some(hash.clone()),
metadata: metadata.clone(),
plan_options: plan_options.clone(),
config_mode: self.config_mode.clone(),
config_mode: self.config_mode_hash.clone(),
},
)
.take(count)
Expand Down Expand Up @@ -249,7 +263,7 @@ where
hash: None,
metadata: CacheKeyMetadata::default(),
plan_options: PlanOptions::default(),
config_mode: self.config_mode.clone(),
config_mode: self.config_mode_hash.clone(),
});
}
}
Expand Down Expand Up @@ -284,7 +298,7 @@ where
schema_id: Arc::clone(&self.schema.schema_id),
metadata,
plan_options,
config_mode: self.config_mode.clone(),
config_mode: self.config_mode_hash.clone(),
};

if experimental_reuse_query_plans {
Expand Down Expand Up @@ -490,7 +504,7 @@ where
schema_id: Arc::clone(&self.schema.schema_id),
metadata,
plan_options,
config_mode: self.config_mode.clone(),
config_mode: self.config_mode_hash.clone(),
};

let context = request.context.clone();
Expand Down Expand Up @@ -632,20 +646,20 @@ fn stats_report_key_hash(stats_report_key: &str) -> String {
hex::encode(result)
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(crate) struct CachingQueryKey {
pub(crate) query: String,
pub(crate) schema_id: Arc<String>,
pub(crate) operation: Option<String>,
pub(crate) hash: Arc<QueryHash>,
pub(crate) metadata: CacheKeyMetadata,
pub(crate) plan_options: PlanOptions,
pub(crate) config_mode: ConfigMode,
pub(crate) config_mode: Arc<QueryHash>,
}

// Update this key every time the cache key or the query plan format has to change.
// When changed it MUST BE CALLED OUT PROMINENTLY IN THE CHANGELOG.
const CACHE_KEY_VERSION: usize = 0;
const CACHE_KEY_VERSION: usize = 1;
const FEDERATION_VERSION: &str = std::env!("FEDERATION_VERSION");

impl std::fmt::Display for CachingQueryKey {
Expand All @@ -654,42 +668,57 @@ impl std::fmt::Display for CachingQueryKey {
hasher.update(self.operation.as_deref().unwrap_or("-"));
let operation = hex::encode(hasher.finalize());

let mut hasher = Sha256::new();
hasher.update(serde_json::to_vec(&self.metadata).expect("serialization should not fail"));
hasher
.update(serde_json::to_vec(&self.plan_options).expect("serialization should not fail"));
hasher
.update(serde_json::to_vec(&self.config_mode).expect("serialization should not fail"));
hasher.update(&*self.schema_id);
let mut hasher = StructHasher::new();
"^metadata".hash(&mut hasher);
self.metadata.hash(&mut hasher);
"^plan_options".hash(&mut hasher);
self.plan_options.hash(&mut hasher);
"^config_mode".hash(&mut hasher);
self.config_mode.hash(&mut hasher);
let metadata = hex::encode(hasher.finalize());

write!(
f,
"plan:{}:{}:{}:{}:{}",
"plan:cache:{}:federation:{}:{}:opname:{}:metadata:{}",
CACHE_KEY_VERSION, FEDERATION_VERSION, self.hash, operation, metadata,
)
}
}

impl Hash for CachingQueryKey {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.schema_id.hash(state);
self.hash.0.hash(state);
self.operation.hash(state);
self.metadata.hash(state);
self.plan_options.hash(state);
self.config_mode.hash(state);
}
}

#[derive(Debug, Clone, Hash, PartialEq, Eq)]
pub(crate) struct WarmUpCachingQueryKey {
pub(crate) query: String,
pub(crate) operation_name: Option<String>,
pub(crate) hash: Option<Arc<QueryHash>>,
pub(crate) metadata: CacheKeyMetadata,
pub(crate) plan_options: PlanOptions,
pub(crate) config_mode: ConfigMode,
pub(crate) config_mode: Arc<QueryHash>,
}

struct StructHasher {
hasher: Sha256,
}

impl StructHasher {
fn new() -> Self {
Self {
hasher: Sha256::new(),
}
}
fn finalize(self) -> Vec<u8> {
self.hasher.finalize().as_slice().into()
}
}

impl Hasher for StructHasher {
fn finish(&self) -> u64 {
unreachable!()
}

fn write(&mut self, bytes: &[u8]) {
self.hasher.update(&[0xFF][..]);
self.hasher.update(bytes);
}
}

impl ValueType for Result<QueryPlannerContent, Arc<QueryPlannerError>> {
Expand Down
12 changes: 6 additions & 6 deletions apollo-router/tests/integration/redis.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async fn query_planner_cache() -> Result<(), BoxError> {
}
// If this test fails and the cache key format changed you'll need to update the key here.
// Look at the top of the file for instructions on getting the new cache key.
let known_cache_key = "plan:0:v2.9.3:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:68e167191994b73c1892549ef57d0ec4cd76d518fad4dac5350846fe9af0b3f1";
let known_cache_key = "plan:cache:1:federation:v2.9.3:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0ade8e18db172d9d51b36a2112513c15032d103100644df418a50596de3adfba";

let config = RedisConfig::from_url("redis://127.0.0.1:6379").unwrap();
let client = RedisClient::new(config, None, None, None);
Expand Down Expand Up @@ -964,7 +964,7 @@ async fn connection_failure_blocks_startup() {
async fn query_planner_redis_update_query_fragments() {
test_redis_query_plan_config_update(
include_str!("fixtures/query_planner_redis_config_update_query_fragments.router.yaml"),
"plan:0:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:d239cf1d493e71f4bcb05e727c38e4cf55b32eb806791fa415bb6f6c8e5352e5",
"plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:1cfc840090ac76a98f8bd51442f41fd6ca4c8d918b3f8d87894170745acf0734",
)
.await;
}
Expand Down Expand Up @@ -994,7 +994,7 @@ async fn query_planner_redis_update_defer() {
// test just passes locally.
test_redis_query_plan_config_update(
include_str!("fixtures/query_planner_redis_config_update_defer.router.yaml"),
"plan:0:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:752b870a0241594f54b7b593f16ab6cf6529eb5c9fe3d24e6bc4a618c24a5b81",
"plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:2f7fb939d2a8fc978e5a4e9d17998074fc30366dcc673236237a885819084fc0",
)
.await;
}
Expand All @@ -1016,7 +1016,7 @@ async fn query_planner_redis_update_type_conditional_fetching() {
include_str!(
"fixtures/query_planner_redis_config_update_type_conditional_fetching.router.yaml"
),
"plan:0:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:e2145b320a44bebbd687c714dcfd046c032e56fe394aedcf50d9ab539f4354ea",
"plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0fd0a376f59f0565768ea5ad8eadfbbf60d64c593c807457a0776d2f39773a25",
)
.await;
}
Expand All @@ -1038,7 +1038,7 @@ async fn query_planner_redis_update_reuse_query_fragments() {
include_str!(
"fixtures/query_planner_redis_config_update_reuse_query_fragments.router.yaml"
),
"plan:0:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:8b6c1838a55cbc6327adb5507f103eed1d5b1071e9acb9c67e098c5b9ea2887e",
"plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:3f30f0e2d149d00c9370c8046e4dd5f23d6ceb6f05a6cf06d5eb021510564248",
)
.await;
}
Expand All @@ -1063,7 +1063,7 @@ async fn test_redis_query_plan_config_update(updated_config: &str, new_cache_key
router.clear_redis_cache().await;

// If the tests above are failing, this is the key that needs to be changed first.
let starting_key = "plan:0:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:41ae54204ebb1911412cf23e8f1d458cb08d6fabce16f255f7a497fd2b6fe213";
let starting_key = "plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0ade8e18db172d9d51b36a2112513c15032d103100644df418a50596de3adfba";
assert_ne!(starting_key, new_cache_key, "starting_key (cache key for the initial config) and new_cache_key (cache key with the updated config) should not be equal. This either means that the cache key is not being generated correctly, or that the test is not actually checking the updated key.");

router.execute_default_query().await;
Expand Down

0 comments on commit 71209e3

Please sign in to comment.