refactor query planner (#1233)

* move the query cache into the query planner

* move introspection in the query planner

* keep the known introspection queries but remove the mutable introspection cache

we pregenerate introspection responses for known queries, but we will
rely on the caching query planner to store all the actually received
introspection queries

* remove QueryCache

we are now relying on the caching query planner

Co-authored-by: Gary Pennington <gary@apollographql.com>

apollo-router/src/error.rs

@@ -5,7 +5,10 @@ use crate::*;
 use displaydoc::Display;
 use miette::{Diagnostic, NamedSource, Report, SourceSpan};
 pub use router_bridge::planner::{PlanError, PlannerError};
-use router_bridge::planner::{PlanErrors, UsageReporting};
+use router_bridge::{
+    introspect::IntrospectionError,
+    planner::{PlanErrors, UsageReporting},
+};
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 use thiserror::Error;
@@ -276,6 +279,9 @@ pub enum QueryPlannerError {
 
     /// spec error: {0}
     SpecError(SpecError),
+
+    /// introspection error: {0}
+    Introspection(IntrospectionError),
 }
 
 #[derive(Clone, Debug, Error)]

apollo-router/src/introspection.rs

@@ -3,9 +3,8 @@ use include_dir::include_dir;
 use once_cell::sync::Lazy;
 use router_bridge::introspect::{self, IntrospectionError};
 use std::collections::HashMap;
-use tokio::sync::RwLock;
 
-/// KNOWN_INTROSPECTION_QUERIES we will serve through NaiveIntrospection.
+/// KNOWN_INTROSPECTION_QUERIES we will serve through Introspection.
 ///
 /// If you would like to add one, put it in the "well_known_introspection_queries" folder.
 static KNOWN_INTROSPECTION_QUERIES: Lazy<Vec<String>> = Lazy::new(|| {
@@ -26,24 +25,22 @@ static KNOWN_INTROSPECTION_QUERIES: Lazy<Vec<String>> = Lazy::new(|| {
 
 /// A cache containing our well known introspection queries.
 #[derive(Debug)]
-pub struct Introspection {
-    cache: RwLock<HashMap<String, Response>>,
+pub(crate) struct Introspection {
+    cache: HashMap<String, Response>,
 }
 
 impl Introspection {
     #[cfg(test)]
-    pub fn from_cache(cache: HashMap<String, Response>) -> Self {
-        Self {
-            cache: RwLock::new(cache),
-        }
+    pub(crate) fn from_cache(cache: HashMap<String, Response>) -> Self {
+        Self { cache }
     }
 
-    /// Create a `NaiveIntrospection` from a `Schema`.
+    /// Create a `Introspection` from a `Schema`.
     ///
     /// This function will populate a cache in a blocking manner.
-    /// This is why `NaiveIntrospection` instanciation happens in a spawn_blocking task on the state_machine side.
-    pub fn from_schema(schema: &Schema) -> Self {
-        let span = tracing::trace_span!("naive_introspection_population");
+    /// This is why `Introspection` instanciation happens in a spawn_blocking task on the state_machine side.
+    pub(crate) fn from_schema(schema: &Schema) -> Self {
+        let span = tracing::trace_span!("introspection_population");
         let _guard = span.enter();
 
         let cache = introspect::batch_introspect(
@@ -89,26 +86,19 @@ impl Introspection {
         })
         .unwrap_or_default();
 
-        Self {
-            cache: RwLock::new(cache),
-        }
-    }
-
-    /// Get a cached response for a query.
-    pub async fn get(&self, query: &str) -> Option<Response> {
-        self.cache
-            .read()
-            .await
-            .get(query)
-            .map(std::clone::Clone::clone)
+        Self { cache }
     }
 
     /// Execute an introspection and cache the response.
-    pub async fn execute(
+    pub(crate) async fn execute(
         &self,
         schema_sdl: &str,
         query: &str,
     ) -> Result<Response, IntrospectionError> {
+        if let Some(response) = self.cache.get(query) {
+            return Ok(response.clone());
+        }
+
         // Do the introspection query and cache it
         let mut response = introspect::batch_introspect(schema_sdl, vec![query.to_owned()])
             .map_err(|err| IntrospectionError {
@@ -132,33 +122,29 @@ impl Introspection {
             })?;
         let response = Response::builder().data(introspection_result).build();
 
-        self.cache
-            .write()
-            .await
-            .insert(query.to_string(), response.clone());
-
         Ok(response)
     }
 }
 
 #[cfg(test)]
-mod naive_introspection_tests {
+mod introspection_tests {
     use super::*;
 
     #[tokio::test]
     async fn test_plan() {
         let query_to_test = "this is a test query";
+        let schema = " ";
         let expected_data = Response::builder().data(42).build();
 
         let cache = [(query_to_test.into(), expected_data.clone())]
             .iter()
             .cloned()
             .collect();
-        let naive_introspection = Introspection::from_cache(cache);
+        let introspection = Introspection::from_cache(cache);
 
         assert_eq!(
             expected_data,
-            naive_introspection.get(query_to_test).await.unwrap()
+            introspection.execute(schema, query_to_test).await.unwrap()
         );
     }
 

apollo-router/src/lib.rs

@@ -41,7 +41,6 @@ pub mod layers;
 #[macro_use]
 pub mod plugin;
 pub mod plugins;
-mod query_cache;
 mod query_planner;
 mod reload;
 mod request;

apollo-router/src/plugin/test/mod.rs

@@ -11,7 +11,6 @@ pub use service::{
 use crate::introspection::Introspection;
 use crate::layers::DEFAULT_BUFFER_SIZE;
 use crate::plugin::Plugin;
-use crate::query_cache::QueryCache;
 use crate::query_planner::BridgeQueryPlanner;
 use crate::query_planner::CachingQueryPlanner;
 use crate::services::layers::apq::APQLayer;
@@ -131,7 +130,11 @@ impl PluginTestHarness {
         let schema = Arc::new(Schema::from(schema));
 
         let query_planner = CachingQueryPlanner::new(
-            BridgeQueryPlanner::new(schema.clone()).await?,
+            BridgeQueryPlanner::new(
+                schema.clone(),
+                Some(Arc::new(Introspection::from_schema(&schema))),
+            )
+            .await?,
             DEFAULT_BUFFER_SIZE,
         )
         .boxed();
@@ -172,8 +175,6 @@ impl PluginTestHarness {
                                         DEFAULT_BUFFER_SIZE,
                                     ))
                                     .schema(schema.clone())
-                                    .query_cache(Arc::new(QueryCache::new(0, schema.clone())))
-                                    .introspection(Arc::new(Introspection::from_schema(&schema)))
                                     .build(),
                             )
                         }),