Skip to content

Commit

Permalink
fix: clone value instead taking ownership for dedupe (#3007)
Browse files Browse the repository at this point in the history
  • Loading branch information
laststylebender14 authored Oct 14, 2024
1 parent a7bc152 commit 37ad714
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
3 changes: 2 additions & 1 deletion src/core/app_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::sync::Arc;
use async_graphql::dynamic::{self, DynamicRequest};
use async_graphql_value::ConstValue;

use super::lift::Lift;
use crate::core::async_graphql_hyper::OperationId;
use crate::core::auth::context::GlobalAuthContext;
use crate::core::blueprint::{Blueprint, Definition, SchemaModifiers};
Expand All @@ -26,7 +27,7 @@ pub struct AppContext {
pub endpoints: EndpointSet<Checked>,
pub auth_ctx: Arc<GlobalAuthContext>,
pub dedupe_handler: Arc<DedupeResult<IoId, ConstValue, Error>>,
pub dedupe_operation_handler: DedupeResult<OperationId, Arc<async_graphql::Response>, Error>,
pub dedupe_operation_handler: DedupeResult<OperationId, Lift<async_graphql::Response>, Error>,
}

impl AppContext {
Expand Down
22 changes: 16 additions & 6 deletions src/core/jit/graphql_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::BTreeMap;
use std::future::Future;
use std::sync::Arc;

use async_graphql::{Data, Executor, Response, ServerError, Value};
use async_graphql::{Data, Executor, Response, Value};
use async_graphql_value::{ConstValue, Extensions};
use futures_util::stream::BoxStream;

Expand All @@ -11,6 +11,7 @@ use crate::core::async_graphql_hyper::OperationId;
use crate::core::http::RequestContext;
use crate::core::jit;
use crate::core::jit::ConstValueExecutor;
use crate::core::lift::{CanLift, Lift};
use crate::core::merge_right::MergeRight;

#[derive(Clone)]
Expand Down Expand Up @@ -64,14 +65,23 @@ impl JITExecutor {
.dedupe(&self.operation_id, || {
Box::pin(async move {
let resp = self.exec(exec, jit_request).await;
Ok(Arc::new(resp))
Ok(resp.lift())
})
})
.await;
let val = out.unwrap_or_default();
Arc::into_inner(val).unwrap_or_else(|| {
Response::from_errors(vec![ServerError::new("Deduplication failed", None)])
})

out.map(|response| response.take()).unwrap_or_default()
}
}

impl Clone for Lift<async_graphql::Response> {
fn clone(&self) -> Self {
let mut res = async_graphql::Response::new(self.data.clone())
.cache_control(self.cache_control)
.http_headers(self.http_headers.clone());
res.errors = self.errors.clone();
res.extensions = self.extensions.clone();
res.into()
}
}

Expand Down

1 comment on commit 37ad714

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 9.18ms 4.00ms 41.11ms 69.89%
Req/Sec 2.76k 299.22 3.03k 95.08%

329574 requests in 30.01s, 1.65GB read

Requests/sec: 10980.84

Transfer/sec: 56.36MB

Please sign in to comment.