Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: dedupe dedupe settings #2183

Merged
merged 5 commits into from
Jun 14, 2024
Merged

fix: dedupe dedupe settings #2183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70c8fe3
Restructure dedupe settings
bnchi Jun 13, 2024
0de5a57
Try fixing lint warnings
bnchi Jun 13, 2024
808bc43
Merge branch 'main' into feat/restructre-deque-settings
bnchi Jun 13, 2024
4890980
update docs
tusharmath Jun 13, 2024
0740a43
remove duplicate check
bnchi Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions generated/.tailcallrc.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down Expand Up @@ -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
Expand Down
22 changes: 4 additions & 18 deletions generated/.tailcallrc.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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": [
Expand Down
4 changes: 2 additions & 2 deletions src/core/blueprint/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
}
},
)
Expand Down
4 changes: 0 additions & 4 deletions src/core/blueprint/upstream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down Expand Up @@ -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()
Expand Down
15 changes: 9 additions & 6 deletions src/core/config/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down
19 changes: 0 additions & 19 deletions src/core/config/upstream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/http/request_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
47 changes: 12 additions & 35 deletions src/core/ir/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,45 +66,22 @@
) -> 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

Check warning on line 81 in src/core/ir/io.rs

View check run for this annotation

Codecov / codecov/patch

src/core/ir/io.rs#L81 

Added line #L81 was not covered by tests
})
})
.await

Check warning on line 84 in src/core/ir/io.rs

View check run for this annotation

Codecov / codecov/patch

src/core/ir/io.rs#L84 

Added line #L84 was not covered by tests
})
} else {
self.eval_inner(ctx)
Expand Down
4 changes: 2 additions & 2 deletions tests/core/snapshots/async-cache-enable-multiple-resolvers.md_merged.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/core/snapshots/async-cache-enabled.md_merged.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/core/snapshots/async-cache-global.md_merged.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/core/snapshots/async-cache-inflight-request.md_merged.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/core/snapshots/dedupe_batch_query_execution.md_merged.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/execution/async-cache-enable-multiple-resolvers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/execution/async-cache-enabled.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/execution/async-cache-global.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/execution/async-cache-inflight-request.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/execution/dedupe_batch_query_execution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading