fix: improve batching behavior in the dataloader #2174
Conversation
…192-fix-dataloader-batching
WalkthroughUpdated github.com/wundergraph/graphql-go-tools/v2 dependency from v2.0.0-rc.220 to v2.0.0-rc.222 in two go.mod files: router/go.mod and router-tests/go.mod. No other changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Noroth
requested review from
StarpTech,
devsergiy and
jensneuse
as code owners
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
router-tests/go.sumis excluded by!**/*.sumrouter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
router-tests/go.mod(1 hunks)router/go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: StarpTech
PR: wundergraph/cosmo#2157
File: router-tests/go.mod:16-16
Timestamp: 2025-08-20T22:13:25.222Z
Learning: github.com/mark3labs/mcp-go v0.38.0 has regressions and should not be used in the wundergraph/cosmo project. v0.36.0 is the stable version that should be used across router-tests and other modules.
📚 Learning: 2025-08-20T22:13:25.222Z
Learnt from: StarpTech
PR: wundergraph/cosmo#2157
File: router-tests/go.mod:16-16
Timestamp: 2025-08-20T22:13:25.222Z
Learning: github.com/mark3labs/mcp-go v0.38.0 has regressions and should not be used in the wundergraph/cosmo project. v0.36.0 is the stable version that should be used across router-tests and other modules.
Applied to files:
router/go.modrouter-tests/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_push_image
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: integration_test (./events)
- GitHub Check: Analyze (go)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
🔇 Additional comments (1)
router-tests/go.mod (1)
30-30: Tests module mirrors the bump — good; please lock in a regression test for batching.Nice keeping router-tests in sync with router on graphql-go-tools. Add an integration test that exercises the dataloader’s batching window and size limits to prevent regressions.
| github.com/tidwall/sjson v1.2.5 | ||
| github.com/twmb/franz-go v1.16.1 | ||
| github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.220 | ||
| github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.222 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Does rc.222 actually contain the “improve dataloader batching” fix? Add proof + tests.
The PR title/objective mentions batching behavior in the dataloader, but the code change is only a dependency bump. Please:
- Link the graphql-go-tools v2 changelog/commit(s) showing the batching fix introduced between rc.220 → rc.222.
- Add/extend router integration tests to assert the batching semantics (e.g., requests within N ms get coalesced; cap sizes respected; no starvation).
- Ensure no stray replace overrides for graphql-go-tools remain.
🏁 Script executed:
#!/bin/bash
# Verify version bumps and stray replaces.
rg -n 'github.com/wundergraph/graphql-go-tools/v2 v2\.0\.0-rc\.222' router/go.mod router-tests/go.mod
rg -nP '^\s*replace\s+github\.com/wundergraph/graphql-go-tools/v2' -nLength of output: 431
🌐 Web query:
Provide links to release notes/commits in github.com/wundergraph/graphql-go-tools/v2 that describe dataloader batching changes between v2.0.0-rc.220 and v2.0.0-rc.222.
💡 Result:
Here are the relevant GitHub pages (release notes + compare) for v2.0.0-rc.220 → v2.0.0-rc.222 — I couldn't find any commit or release-note text in that range explicitly mentioning “dataloader” or “batching”:
- v2.0.0-rc.220 release notes. [1]
- v2.0.0-rc.222 release notes. [2]
- Compare view (commits between v2.0.0-rc.220 and v2.0.0-rc.222). [3]
If you want, I can scan each commit in the compare and provide direct commit links for any files that touch batching-related code (e.g., packages under v2/pkg/engine/resolve or loader code).
Sources:
[1] https://github.com/wundergraph/graphql-go-tools/releases/tag/v2.0.0-rc.220. (github.com)
[2] https://github.com/wundergraph/graphql-go-tools/releases/tag/v2.0.0-rc.222. (github.com)
[3] wundergraph/graphql-go-tools@v2.0.0-rc.220...v2.0.0-rc.222. (github.com)
Citations:
- 1: https://github.com/wundergraph/graphql-go-tools/releases
- 2: https://github.com/wundergraph/graphql-go-tools/releases
- 3: wundergraph/graphql-go-tools@v2.0.0-rc.220...v2.0.0-rc.222
Link batching fix commit, add tests, remove stray replace
- No batching fix found in v2.0.0-rc.220→rc.222 release notes or commits–please link the specific commit(s) or changelog entry introducing the dataloader batching improvements.
- Add/extend integration tests in router to verify dataloader coalescing behavior (batch window timing, max batch size, starvation prevention).
- Remove the stray
replace github.com/wundergraph/graphql-go-tools/v2 => ../../graphql-go-tools/v2in router/README.md line 33.
🤖 Prompt for AI Agents
In router/go.mod around line 34, the PR updates graphql-go-tools to
v2.0.0-rc.222 but the review requests (1) add a link to the exact commit or
changelog entry that introduced the dataloader batching fix — update the PR
description or go.mod comment with the commit hash or changelog URL (or add a
short note in router/CHANGELOG.md) referencing the specific commit(s) or release
notes; (2) add/extend router integration tests that assert dataloader coalescing
behavior: write tests covering batch window timing, max batch size enforcement,
and starvation prevention (simulate concurrent requests over time and verify
that requests are batched appropriately and no single request/starvation
occurs); and (3) remove the stray replace line in router/README.md (remove the
line `replace github.com/wundergraph/graphql-go-tools/v2 =>
../../graphql-go-tools/v2` at README.md line 33) so the module uses the intended
module proxy version.
|
Merged in #2141 |
Summary by CodeRabbit
Checklist