Distributed query plan cache keys include the Router version number (#…

…6406)
apollographql · Dec 6, 2024 · a201cf5 · a201cf5
1 parent 9ef8296
commit a201cf5
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 69 deletions.
diff --git a/.changesets/config_simon_router_version_in_cache_key.md b/.changesets/config_simon_router_version_in_cache_key.md
@@ -0,0 +1,7 @@
+### Distributed query plan cache keys include the Router version number ([PR #6406](https://github.com/apollographql/router/pull/6406))
+
+More often than not, an Apollo Router release may contain changes that affect what query plans are generated or how they’re represented. To avoid using outdated entries from distributed cache, the cache key includes a counter that was manually incremented with relevant data structure or algorithm changes. Instead the cache key now includes the Router version number, so that different versions will always use separate cache entries.
+
+If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions.
+
+By [@SimonSapin](https://github.com/SimonSapin) in https://github.com/apollographql/router/pull/6406
diff --git a/apollo-router/build/main.rs b/apollo-router/build/main.rs
@@ -1,37 +1,5 @@
-use std::fs;
-use std::path::PathBuf;
-
 mod studio;
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let cargo_manifest: serde_json::Value = basic_toml::from_str(
-        &fs::read_to_string(PathBuf::from(&env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"))
-            .expect("could not read Cargo.toml"),
-    )
-    .expect("could not parse Cargo.toml");
-
-    let router_bridge = cargo_manifest
-        .get("dependencies")
-        .expect("Cargo.toml does not contain dependencies")
-        .as_object()
-        .expect("Cargo.toml dependencies key is not an object")
-        .get("router-bridge")
-        .expect("Cargo.toml dependencies does not have an entry for router-bridge");
-    let router_bridge_version = router_bridge
-        .as_str()
-        .or_else(|| {
-            router_bridge
-                .as_object()
-                .and_then(|o| o.get("version"))
-                .and_then(|version| version.as_str())
-        })
-        .expect("router-bridge does not have a version");
-
-    let mut it = router_bridge_version.split('+');
-    let _ = it.next();
-    let fed_version = it.next().expect("invalid router-bridge version format");
-
-    println!("cargo:rustc-env=FEDERATION_VERSION={fed_version}");
-
     studio::main()
 }
diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs
@@ -843,9 +843,6 @@ pub(crate) fn metric_rust_qp_init(init_error_kind: Option<&'static str>) {
 
 #[cfg(test)]
 mod tests {
-    use std::fs;
-    use std::path::PathBuf;
-
     use serde_json::json;
     use test_log::test;
     use tower::Service;
@@ -1389,28 +1386,6 @@ mod tests {
             .await
     }
 
-    #[test]
-    fn router_bridge_dependency_is_pinned() {
-        let cargo_manifest: serde_json::Value = basic_toml::from_str(
-            &fs::read_to_string(PathBuf::from(&env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"))
-                .expect("could not read Cargo.toml"),
-        )
-        .expect("could not parse Cargo.toml");
-        let router_bridge_version = cargo_manifest
-            .get("dependencies")
-            .expect("Cargo.toml does not contain dependencies")
-            .as_object()
-            .expect("Cargo.toml dependencies key is not an object")
-            .get("router-bridge")
-            .expect("Cargo.toml dependencies does not have an entry for router-bridge")
-            .as_str()
-            .unwrap_or_default();
-        assert!(
-            router_bridge_version.contains('='),
-            "router-bridge in Cargo.toml is not pinned with a '=' prefix"
-        );
-    }
-
     #[tokio::test]
     async fn test_both_mode() {
         let mut harness = crate::TestHarness::builder()

diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs
@@ -632,10 +632,7 @@ pub(crate) struct CachingQueryKey {
     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 = 1;
-const FEDERATION_VERSION: &str = std::env!("FEDERATION_VERSION");
+const ROUTER_VERSION: &str = env!("CARGO_PKG_VERSION");
 
 impl std::fmt::Display for CachingQueryKey {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -654,8 +651,8 @@ impl std::fmt::Display for CachingQueryKey {
 
         write!(
             f,
-            "plan:cache:{}:federation:{}:{}:opname:{}:metadata:{}",
-            CACHE_KEY_VERSION, FEDERATION_VERSION, self.hash, operation, metadata,
+            "plan:router:{}:{}:opname:{}:metadata:{}",
+            ROUTER_VERSION, self.hash, operation, metadata,
         )
     }
 }

diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs
@@ -51,7 +51,10 @@ 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:cache:1:federation:v2.9.3:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:81b9c296d8ebc8adf4575b83a8d296621fd76de6d12d8c91f4552eda02d1dd9c";
+    let known_cache_key = &format!(
+        "plan:router:{}:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:81b9c296d8ebc8adf4575b83a8d296621fd76de6d12d8c91f4552eda02d1dd9c",
+        env!("CARGO_PKG_VERSION")
+    );
 
     let config = RedisConfig::from_url("redis://127.0.0.1:6379").unwrap();
     let client = RedisClient::new(config, None, None, None);
@@ -988,7 +991,10 @@ async fn query_planner_redis_update_query_fragments() {
     test_redis_query_plan_config_update(
         // This configuration turns the fragment generation option *off*.
         include_str!("fixtures/query_planner_redis_config_update_query_fragments.router.yaml"),
-        "plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:a55ce3338ce6d5b78566be89cc6a7ad3fe8a7eeb38229d14ddf647edef84e545",
+        &format!(
+            "plan:router:{}:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:a55ce3338ce6d5b78566be89cc6a7ad3fe8a7eeb38229d14ddf647edef84e545",
+            env!("CARGO_PKG_VERSION")
+        ),
     )
     .await;
 }
@@ -1018,7 +1024,10 @@ 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:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0497501d3d01d05ad142938e7b8d8e7ea13e648aabbbedb47f6291ca8b3e536d",
+        &format!(
+            "plan:router:{}:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:0497501d3d01d05ad142938e7b8d8e7ea13e648aabbbedb47f6291ca8b3e536d",
+            env!("CARGO_PKG_VERSION")
+        ),
     )
     .await;
 }
@@ -1040,7 +1049,10 @@ async fn query_planner_redis_update_type_conditional_fetching() {
         include_str!(
             "fixtures/query_planner_redis_config_update_type_conditional_fetching.router.yaml"
         ),
-        "plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:662f5041882b3f621aeb7bad8e18818173eb077dc4343e16f3a34d2b6b6e4e59",
+        &format!(
+            "plan:router:{}:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:662f5041882b3f621aeb7bad8e18818173eb077dc4343e16f3a34d2b6b6e4e59",
+            env!("CARGO_PKG_VERSION")
+        ),
     )
     .await;
 }
@@ -1063,7 +1075,10 @@ async fn query_planner_redis_update_reuse_query_fragments() {
         include_str!(
             "fixtures/query_planner_redis_config_update_reuse_query_fragments.router.yaml"
         ),
-        "plan:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:9af18c8afd568c197050fc1a60c52a8c98656f1775016110516fabfbedc135fe",
+        &format!(
+            "plan:router:{}:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:9af18c8afd568c197050fc1a60c52a8c98656f1775016110516fabfbedc135fe",
+            env!("CARGO_PKG_VERSION")
+        ),
     )
     .await;
 }
@@ -1088,7 +1103,10 @@ 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:cache:1:federation:v2.9.3:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:81b9c296d8ebc8adf4575b83a8d296621fd76de6d12d8c91f4552eda02d1dd9c";
+    let starting_key = &format!(
+        "plan:router:{}:5938623f2155169070684a48be1e0b8468d0f2c662b5527a2247f683173f7d05:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:81b9c296d8ebc8adf4575b83a8d296621fd76de6d12d8c91f4552eda02d1dd9c",
+        env!("CARGO_PKG_VERSION")
+    );
     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;