From 70c8fe33d5e5a72a0b75faf416c4c0fc1b5dee37 Mon Sep 17 00:00:00 2001 From: Bnchi Date: Thu, 13 Jun 2024 15:43:30 +0300 Subject: [PATCH 1/4] Restructure dedupe settings --- generated/.tailcallrc.graphql | 20 ++------ generated/.tailcallrc.schema.json | 22 ++------- src/core/blueprint/server.rs | 4 +- src/core/blueprint/upstream.rs | 4 -- src/core/config/server.rs | 6 +-- src/core/config/upstream.rs | 19 -------- src/core/http/request_handler.rs | 2 +- src/core/ir/io.rs | 47 ++++++------------- ...e-enable-multiple-resolvers.md_merged.snap | 4 +- .../async-cache-enabled.md_merged.snap | 4 +- .../async-cache-global.md_merged.snap | 4 +- ...sync-cache-inflight-request.md_merged.snap | 4 +- ...edupe_batch_query_execution.md_merged.snap | 4 +- .../async-cache-enable-multiple-resolvers.md | 4 +- tests/execution/async-cache-enabled.md | 4 +- tests/execution/async-cache-global.md | 4 +- .../execution/async-cache-inflight-request.md | 4 +- .../execution/dedupe_batch_query_execution.md | 4 +- 18 files changed, 50 insertions(+), 114 deletions(-) diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql index 19ea5770a5..5c1a8e442f 100644 --- a/generated/.tailcallrc.graphql +++ b/generated/.tailcallrc.graphql @@ -232,16 +232,16 @@ 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 """ + 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. + """ + dedupe: Boolean + """ `globalResponseTimeout` sets the maximum query duration before termination, acting as a safeguard against long-running queries. """ @@ -360,16 +360,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 index 4bfa1d2d17..a98acac2fc 100644 --- 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": "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.", "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 index 521c0f955c..25cf5a21e5 100644 --- a/src/core/blueprint/server.rs +++ b/src/core/blueprint/server.rs @@ -36,7 +36,7 @@ pub struct Server { pub cors: Option, pub experimental_headers: HashSet, pub auth: Option, - pub batch_execution: bool, + pub dedupe: bool, } /// Mimic of mini_v8::Script that's wasm compatible @@ -151,7 +151,7 @@ impl TryFrom 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 index 0c3da374bf..75a90f5ceb 100644 --- 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, pub http2_only: bool, - pub dedupe: bool, - pub dedupe_in_flight: bool, pub on_request: Option, } @@ -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 index 486adfba72..bf8b15d0a5 100644 --- a/src/core/config/server.rs +++ b/src/core/config/server.rs @@ -33,7 +33,7 @@ pub struct Server { /// 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, + pub dedupe: Option, #[serde(default, skip_serializing_if = "is_default")] /// `headers` contains key-value pairs that are included as default headers @@ -205,8 +205,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 index 471535a73c..11b8c5d896 100644 --- 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, - - #[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, - - #[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, } 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 { self.on_request.clone() } diff --git a/src/core/http/request_handler.rs b/src/core/http/request_handler.rs index a84a9f566c..b22a0d8ba5 100644 --- a/src/core/http/request_handler.rs +++ b/src/core/http/request_handler.rs @@ -107,7 +107,7 @@ pub async fn graphql_request( let graphql_request = serde_json::from_slice::(&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 index c9d1508261..902a8e40e4 100644 --- a/src/core/ir/io.rs +++ b/src/core/ir/io.rs @@ -66,44 +66,27 @@ impl Eval for IO { ) -> Pin> + '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 - }) + if ctx.request_ctx.server.dedupe { + 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, + }) + .await + } else { + self.eval_inner(ctx).await } }) } else { 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 index 71c14d8ddf..fc93f85386 100644 --- 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 index 6122c63f2b..5c670d71b6 100644 --- 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 index b1ec8d3c75..cbf1b19790 100644 --- 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 index f9f81e13a8..5c670d71b6 100644 --- 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 index e644cd2fc1..cbf1b19790 100644 --- 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 index 7756b5b7f0..74e0bcc410 100644 --- 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 index 2f85e72a15..622b4334e3 100644 --- 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 index 2d782e8961..95d21ea045 100644 --- 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 index 43b5af613b..7b9aeb61f2 100644 --- 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 index c40e922188..95d21ea045 100644 --- 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 } From 0de5a57ba7110e62ee5e5d6df49b7c66b9ba7d0a Mon Sep 17 00:00:00 2001 From: Bnchi Date: Thu, 13 Jun 2024 15:55:26 +0300 Subject: [PATCH 2/4] Try fixing lint warnings --- src/core/ir/io.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/ir/io.rs b/src/core/ir/io.rs index 902a8e40e4..a948007444 100644 --- a/src/core/ir/io.rs +++ b/src/core/ir/io.rs @@ -66,10 +66,9 @@ impl Eval for IO { ) -> Pin> + 'a + Send>> { // Note: Handled the case separately for performance reasons. It avoids cache // key generation when it's not required - if (!ctx.request_ctx.server.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 { if ctx.request_ctx.server.dedupe { From 4890980cd7589c4b7476e35417d1364d2ed2788b Mon Sep 17 00:00:00 2001 From: Tushar Mathur Date: Thu, 13 Jun 2024 20:35:30 +0530 Subject: [PATCH 3/4] update docs --- generated/.tailcallrc.graphql | 6 ++++-- generated/.tailcallrc.schema.json | 2 +- src/core/config/server.rs | 9 ++++++--- src/core/ir/io.rs | 3 +-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/generated/.tailcallrc.graphql b/generated/.tailcallrc.graphql index 5c1a8e442f..0f68becffb 100644 --- a/generated/.tailcallrc.graphql +++ b/generated/.tailcallrc.graphql @@ -237,8 +237,10 @@ directive @server( """ batchRequests: 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. + 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 """ diff --git a/generated/.tailcallrc.schema.json b/generated/.tailcallrc.schema.json index a98acac2fc..aa51b9a7c6 100644 --- a/generated/.tailcallrc.schema.json +++ b/generated/.tailcallrc.schema.json @@ -1024,7 +1024,7 @@ ] }, "dedupe": { - "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.", + "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" diff --git a/src/core/config/server.rs b/src/core/config/server.rs index bf8b15d0a5..e336438ba1 100644 --- a/src/core/config/server.rs +++ b/src/core/config/server.rs @@ -30,9 +30,12 @@ pub struct Server { pub batch_requests: Option, #[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. + /// 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, #[serde(default, skip_serializing_if = "is_default")] diff --git a/src/core/ir/io.rs b/src/core/ir/io.rs index a948007444..556bc7d76f 100644 --- a/src/core/ir/io.rs +++ b/src/core/ir/io.rs @@ -72,8 +72,7 @@ impl Eval for IO { if let Some(key) = self.cache_key(&ctx) { Box::pin(async move { if ctx.request_ctx.server.dedupe { - ctx - .request_ctx + ctx.request_ctx .cache .dedupe(&key.clone(), || { Box::pin(async move { From 0740a438f614fbd797a09a5d74a03d8d689aab20 Mon Sep 17 00:00:00 2001 From: Bnchi Date: Thu, 13 Jun 2024 21:19:22 +0300 Subject: [PATCH 4/4] remove duplicate check --- src/core/ir/io.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/core/ir/io.rs b/src/core/ir/io.rs index 556bc7d76f..6e20d85539 100644 --- a/src/core/ir/io.rs +++ b/src/core/ir/io.rs @@ -71,21 +71,17 @@ impl Eval for IO { } if let Some(key) = self.cache_key(&ctx) { Box::pin(async move { - if ctx.request_ctx.server.dedupe { - 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 - }) + 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).await - } + }) + .await }) } else { self.eval_inner(ctx)