fix: dedupe dedupe settings (#2183)

Co-authored-by: Tushar Mathur <tusharmath@gmail.com>
tailcallhq · Jun 14, 2024 · 55070c2 · 55070c2 · github-actions · Jun 14, 2024
1 parent dd098ca
commit 55070c2
Show file tree

Hide file tree

Showing 18 changed files with 55 additions and 120 deletions.
diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql
@@ -232,16 +232,18 @@ directive @server(
   """
   apolloTracing: Boolean
   """
-  When set to `true`, it will ensure no graphQL execution is made more than once if 
-  similar query is being executed across the server's lifetime.
-  """
-  batchExecution: Boolean
-  """
   `batchRequests` combines multiple requests into one, improving performance but potentially 
   introducing latency and complicating debugging. Use judiciously. @default `false`.
   """
   batchRequests: Boolean
   """
+  Enables deduplication of IO operations to enhance performance.This flag prevents 
+  duplicate IO requests from being executed concurrently, reducing resource load. Caution: 
+  May lead to issues with APIs that expect unique results for identical inputs, such 
+  as nonce-based APIs.
+  """
+  dedupe: Boolean
+  """
   `globalResponseTimeout` sets the maximum query duration before termination, acting 
   as a safeguard against long-running queries.
   """
@@ -360,16 +362,6 @@ directive @upstream(
   """
   connectTimeout: Int
   """
-  When set to `true`, it will ensure no HTTP, GRPC, or any other IO call is made more 
-  than once within the context of a single GraphQL request.
-  """
-  dedupe: Boolean
-  """
-  When set to `true`, it will ensure no HTTP, GRPC, or any other IO call is made more 
-  than once if similar request is inflight across the server's lifetime.
-  """
-  dedupeInFlight: Boolean
-  """
   The `http2Only` setting allows you to specify whether the client should always issue 
   HTTP2 requests, without checking if the server supports it or not. By default it 
   is set to `false` for all HTTP requests made by the server, but is automatically 

diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json
@@ -1016,15 +1016,15 @@
             "null"
           ]
         },
-        "batchExecution": {
-          "description": "When set to `true`, it will ensure no graphQL execution is made more than once if similar query is being executed across the server's lifetime.",
+        "batchRequests": {
+          "description": "`batchRequests` combines multiple requests into one, improving performance but potentially introducing latency and complicating debugging. Use judiciously. @default `false`.",
           "type": [
             "boolean",
             "null"
           ]
         },
-        "batchRequests": {
-          "description": "`batchRequests` combines multiple requests into one, improving performance but potentially introducing latency and complicating debugging. Use judiciously. @default `false`.",
+        "dedupe": {
+          "description": "Enables deduplication of IO operations to enhance performance.\n\nThis flag prevents duplicate IO requests from being executed concurrently, reducing resource load. Caution: May lead to issues with APIs that expect unique results for identical inputs, such as nonce-based APIs.",
           "type": [
             "boolean",
             "null"
@@ -1435,20 +1435,6 @@
           "format": "uint64",
           "minimum": 0.0
         },
-        "dedupe": {
-          "description": "When set to `true`, it will ensure no HTTP, GRPC, or any other IO call is made more than once within the context of a single GraphQL request.",
-          "type": [
-            "boolean",
-            "null"
-          ]
-        },
-        "dedupeInFlight": {
-          "description": "When set to `true`, it will ensure no HTTP, GRPC, or any other IO call is made more than once if similar request is inflight across the server's lifetime.",
-          "type": [
-            "boolean",
-            "null"
-          ]
-        },
         "http2Only": {
           "description": "The `http2Only` setting allows you to specify whether the client should always issue HTTP2 requests, without checking if the server supports it or not. By default it is set to `false` for all HTTP requests made by the server, but is automatically set to true for GRPC.",
           "type": [

diff --git a/src/core/blueprint/server.rs b/src/core/blueprint/server.rs
@@ -36,7 +36,7 @@ pub struct Server {
     pub cors: Option<Cors>,
     pub experimental_headers: HashSet<HeaderName>,
     pub auth: Option<Auth>,
-    pub batch_execution: bool,
+    pub dedupe: bool,
 }
 
 /// Mimic of mini_v8::Script that's wasm compatible
@@ -151,7 +151,7 @@ impl TryFrom<crate::core::config::ConfigModule> for Server {
                         script,
                         cors,
                         auth,
-                        batch_execution: config_server.get_batch_execution(),
+                        dedupe: config_server.get_dedupe(),
                     }
                 },
             )

diff --git a/src/core/blueprint/upstream.rs b/src/core/blueprint/upstream.rs
@@ -27,8 +27,6 @@ pub struct Upstream {
     pub http_cache: u64,
     pub batch: Option<Batch>,
     pub http2_only: bool,
-    pub dedupe: bool,
-    pub dedupe_in_flight: bool,
     pub on_request: Option<String>,
 }
 
@@ -83,8 +81,6 @@ impl TryFrom<&ConfigModule> for Upstream {
                 http_cache: (config_upstream).get_http_cache_size(),
                 batch,
                 http2_only: (config_upstream).get_http_2_only(),
-                dedupe: (config_upstream).get_dedupe(),
-                dedupe_in_flight: (config_upstream).get_dedupe_in_flight(),
                 on_request: (config_upstream).get_on_request(),
             })
             .to_result()

diff --git a/src/core/config/server.rs b/src/core/config/server.rs
@@ -30,10 +30,13 @@ pub struct Server {
     pub batch_requests: Option<bool>,
 
     #[serde(default, skip_serializing_if = "is_default")]
-    /// When set to `true`, it will ensure no graphQL execution is made more
-    /// than once if similar query is being executed across the
-    /// server's lifetime.
-    pub batch_execution: Option<bool>,
+    /// Enables deduplication of IO operations to enhance performance.
+    ///
+    /// This flag prevents duplicate IO requests from being executed
+    /// concurrently, reducing resource load. Caution: May lead to issues
+    /// with APIs that expect unique results for identical inputs, such as
+    /// nonce-based APIs.
+    pub dedupe: Option<bool>,
 
     #[serde(default, skip_serializing_if = "is_default")]
     /// `headers` contains key-value pairs that are included as default headers
@@ -205,8 +208,8 @@ impl Server {
         self.pipeline_flush.unwrap_or(true)
     }
 
-    pub fn get_batch_execution(&self) -> bool {
-        self.batch_execution.unwrap_or(false)
+    pub fn get_dedupe(&self) -> bool {
+        self.dedupe.unwrap_or(false)
     }
 }
 

diff --git a/src/core/config/upstream.rs b/src/core/config/upstream.rs
@@ -140,17 +140,6 @@ pub struct Upstream {
     /// The User-Agent header value to be used in HTTP requests. @default
     /// `Tailcall/1.0`
     pub user_agent: Option<String>,
-
-    #[serde(default, skip_serializing_if = "is_default")]
-    /// When set to `true`, it will ensure no HTTP, GRPC, or any other IO call
-    /// is made more than once within the context of a single GraphQL request.
-    pub dedupe: Option<bool>,
-
-    #[serde(default, skip_serializing_if = "is_default")]
-    /// When set to `true`, it will ensure no HTTP, GRPC, or any other IO call
-    /// is made more than once if similar request is inflight across the
-    /// server's lifetime.
-    pub dedupe_in_flight: Option<bool>,
 }
 
 impl Upstream {
@@ -203,14 +192,6 @@ impl Upstream {
         self.http2_only.unwrap_or(false)
     }
 
-    pub fn get_dedupe(&self) -> bool {
-        self.dedupe.unwrap_or(false)
-    }
-
-    pub fn get_dedupe_in_flight(&self) -> bool {
-        self.dedupe_in_flight.unwrap_or(false)
-    }
-
     pub fn get_on_request(&self) -> Option<String> {
         self.on_request.clone()
     }

diff --git a/src/core/http/request_handler.rs b/src/core/http/request_handler.rs
@@ -107,7 +107,7 @@ pub async fn graphql_request<T: DeserializeOwned + GraphQLRequestLike>(
     let graphql_request = serde_json::from_slice::<T>(&bytes);
     match graphql_request {
         Ok(mut request) => {
-            if !(app_ctx.blueprint.server.batch_execution && request.is_query()) {
+            if !(app_ctx.blueprint.server.dedupe && request.is_query()) {
                 Ok(execute_query(&app_ctx, &req_ctx, request).await?)
             } else {
                 let operation_id = request.operation_id(&req.headers);

diff --git a/src/core/ir/io.rs b/src/core/ir/io.rs
@@ -66,45 +66,22 @@ impl Eval for IO {
     ) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
         // Note: Handled the case separately for performance reasons. It avoids cache
         // key generation when it's not required
-        if (!ctx.request_ctx.upstream.dedupe_in_flight && !ctx.request_ctx.upstream.dedupe)
-            || !ctx.is_query()
-        {
+        if !ctx.request_ctx.server.dedupe || !ctx.is_query() {
             return self.eval_inner(ctx);
         }
-
         if let Some(key) = self.cache_key(&ctx) {
             Box::pin(async move {
-                match (
-                    ctx.request_ctx.upstream.dedupe,
-                    ctx.request_ctx.upstream.dedupe_in_flight,
-                ) {
-                    (true, false) => {
-                        ctx.request_ctx
-                            .cache
-                            .dedupe(&key, || Box::pin(self.eval_inner(ctx)))
-                            .await
-                    }
-                    (true, true) => {
-                        ctx.request_ctx
-                            .cache
-                            .dedupe(&key.clone(), || {
-                                Box::pin(async move {
-                                    ctx.request_ctx
-                                        .dedupe_handler
-                                        .dedupe(&key, || Box::pin(self.eval_inner(ctx)))
-                                        .await
-                                })
-                            })
-                            .await
-                    }
-                    (false, true) => {
-                        ctx.request_ctx
-                            .dedupe_handler
-                            .dedupe(&key, || Box::pin(self.eval_inner(ctx)))
-                            .await
-                    }
-                    (false, false) => self.eval_inner(ctx).await,
-                }
+                ctx.request_ctx
+                    .cache
+                    .dedupe(&key.clone(), || {
+                        Box::pin(async move {
+                            ctx.request_ctx
+                                .dedupe_handler
+                                .dedupe(&key, || Box::pin(self.eval_inner(ctx)))
+                                .await
+                        })
+                    })
+                    .await
             })
         } else {
             self.eval_inner(ctx)

diff --git a/tests/core/snapshots/async-cache-enable-multiple-resolvers.md_merged.snap b/tests/core/snapshots/async-cache-enable-multiple-resolvers.md_merged.snap
@@ -3,8 +3,8 @@ source: tests/core/spec.rs
 expression: formatter
 ---
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true) {
+  @server(dedupe: true, port: 8000, queryValidation: false)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/core/snapshots/async-cache-enabled.md_merged.snap b/tests/core/snapshots/async-cache-enabled.md_merged.snap
@@ -3,8 +3,8 @@ source: tests/core/spec.rs
 expression: formatter
 ---
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true) {
+  @server(dedupe: true, port: 8000, queryValidation: false)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/core/snapshots/async-cache-global.md_merged.snap b/tests/core/snapshots/async-cache-global.md_merged.snap
@@ -3,8 +3,8 @@ source: tests/core/spec.rs
 expression: formatter
 ---
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupeInFlight: true) {
+  @server(dedupe: true, port: 8000, queryValidation: false)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/core/snapshots/async-cache-inflight-request.md_merged.snap b/tests/core/snapshots/async-cache-inflight-request.md_merged.snap
@@ -3,8 +3,8 @@ source: tests/core/spec.rs
 expression: formatter
 ---
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true, dedupeInFlight: true) {
+  @server(dedupe: true, port: 8000, queryValidation: false)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/core/snapshots/dedupe_batch_query_execution.md_merged.snap b/tests/core/snapshots/dedupe_batch_query_execution.md_merged.snap
@@ -3,8 +3,8 @@ source: tests/core/spec.rs
 expression: formatter
 ---
 schema
-  @server(batchExecution: true, port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupeInFlight: true) {
+  @server(dedupe: true, port: 8000, queryValidation: false)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/execution/async-cache-enable-multiple-resolvers.md b/tests/execution/async-cache-enable-multiple-resolvers.md
@@ -2,8 +2,8 @@
 
 ```graphql @config
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true) {
+  @server(port: 8000, queryValidation: false, dedupe: true)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/execution/async-cache-enabled.md b/tests/execution/async-cache-enabled.md
@@ -2,8 +2,8 @@
 
 ```graphql @config
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true) {
+  @server(port: 8000, queryValidation: false, dedupe: true)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/execution/async-cache-global.md b/tests/execution/async-cache-global.md
@@ -2,8 +2,8 @@
 
 ```graphql @config
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupeInFlight: true) {
+  @server(port: 8000, queryValidation: false, dedupe: true)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/execution/async-cache-inflight-request.md b/tests/execution/async-cache-inflight-request.md
@@ -2,8 +2,8 @@
 
 ```graphql @config
 schema
-  @server(port: 8000, queryValidation: false)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupe: true, dedupeInFlight: true) {
+  @server(port: 8000, queryValidation: false, dedupe: true)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
 

diff --git a/tests/execution/dedupe_batch_query_execution.md b/tests/execution/dedupe_batch_query_execution.md
@@ -2,8 +2,8 @@
 
 ```graphql @config
 schema
-  @server(port: 8000, queryValidation: false, batchExecution: true)
-  @upstream(baseURL: "http://jsonplaceholder.typicode.com", dedupeInFlight: true) {
+  @server(port: 8000, queryValidation: false, dedupe: true)
+  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
   query: Query
 }
Thread Stats	Avg	Stdev	Max	+/- Stdev
Latency	6.67ms	2.89ms	94.49ms	71.28%
Req/Sec	3.78k	188.67	4.24k	90.00%