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.

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

Adds Variable Batching #6981

Merged
merged 43 commits into from May 22, 2024
Merged

Adds Variable Batching #6981

merged 43 commits into from May 22, 2024

Conversation

michaelstaib
Copy link
Member

@michaelstaib michaelstaib commented Mar 9, 2024

Variable Batching addresses a specific batching use case by allowing a single request to carry multiple sets of variables, potentially enabling more optimized execution paths through the executor. In experimentations we could reduce the batching overhead to the impact a DataLoder has on a request, which is promising.

{
  "query": "query getHero($a: Int!, $b: String!) { field(a: $a, b: $b) }",
  "variables": [
    {
      "a": 1,
      "b": "abc"
    },
    {
      "a": 2,
      "b": "def"
    }
  ]
}
  • Rework Core Request and Result Types
  • Remove Apollo Tracing
  • Remove Cost Control
  • Remove Operation Batching
  • Remove Apollo Style Batching Implementation
  • Add Variable Batching
  • Add Request Batching
  • Fix MultiPart Batching
  • Add Tests for Variable Batching
  • Add Tests for Request Batching
  • Add Tests for Variable Batching in combination with Request Batching
  • Check side effects with combined batches

Follow-up Tasks

  • Add Apollo MultiPart Subscription Formatter
    • Add Tests for Variable Batching in combination with Multipart Batching
    • Add Tests for MultiPart Batching
  • Add IBM Cost Control System

@michaelstaib michaelstaib changed the title Start refactoring request/response structures. Adds Variable Batching Mar 9, 2024
Copy link

github-actions bot commented Mar 9, 2024

Qodana for .NET

9 new problems were found

Inspection name Severity Problems
Redundant argument with default value 🔶 Warning 3
Redundant using directive 🔶 Warning 3
Possible unassigned object created by 'new' expression 🔶 Warning 2
Namespace does not correspond to file location 🔶 Warning 1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Copy link

codecov bot commented Mar 17, 2024

Codecov Report

Attention: Patch coverage is 86.73835% with 111 lines in your changes are missing coverage. Please review.

Project coverage is 77.10%. Comparing base (926160a) to head (5a28e03).
Report is 3 commits behind head on main.

Current head 5a28e03 differs from pull request most recent head 9bd8a89

Please upload reports for the commit 9bd8a89 to get more accurate results.

Files Patch % Lines
...Abstractions/Serialization/Utf8JsonWriterHelper.cs 73.03% 24 Missing ⚠️
...src/Execution/Serialization/JsonResultFormatter.cs 81.81% 14 Missing ⚠️
...tCore/src/Transport.Http/HttpResources.Designer.cs 0.00% 10 Missing ⚠️
...ormatters/OperationResultSnapshotValueFormatter.cs 52.94% 8 Missing ⚠️
...cution/Serialization/EventStreamResultFormatter.cs 88.73% 8 Missing ⚠️
...ore/src/Transport.Http/DefaultGraphQLHttpClient.cs 77.41% 7 Missing ⚠️
...Core/Serialization/DefaultHttpResponseFormatter.cs 87.23% 6 Missing ⚠️
...late/AspNetCore/src/AspNetCore/MiddlewareHelper.cs 69.23% 4 Missing ⚠️
...Execution/Pipeline/OperationExecutionMiddleware.cs 95.87% 4 Missing ⚠️
...xecution/Serialization/MultiPartResultFormatter.cs 93.22% 4 Missing ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6981      +/-   ##
==========================================
- Coverage   79.68%   77.10%   -2.59%     
==========================================
  Files        2631     2648      +17     
  Lines      131990   132555     +565     
==========================================
- Hits       105177   102206    -2971     
- Misses      26813    30349    +3536     
Flag Coverage Δ
unittests 77.10% <86.73%> (-2.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented May 1, 2024

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

@michaelstaib michaelstaib merged commit a162880 into main May 22, 2024
3 checks passed
@michaelstaib michaelstaib deleted the mst/variable-batching branch May 22, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant