perf: use singleflight for more db queries to better handle many similar parallel requests#3023
perf: use singleflight for more db queries to better handle many similar parallel requests#3023maxmanuylov wants to merge 1 commit into
Conversation
β¦lar parallel requests
|
All contributors have signed the CLA βοΈ β
|
π WalkthroughWalkthroughDataReader in the singleflight proxy is extended with separate singleflight groups for QueryRelationships, QuerySingleAttribute, and QueryAttributes, adding key-builder and iterator-drain helpers. HeadSnapshot now uses a renamed field. Test mocks and suites are expanded to cover the new deduplication behavior. ChangesSingleflight-backed query deduplication
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 warning)
β Passed checks (4 passed)
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Actionable comments posted: 2
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/storage/proxies/singleflight/data_reader_test.go`:
- Around line 385-390: The test goroutine in data_reader_test.go is calling
Ginkgo assertions inside go func() without recovery, which can make failures
escape the spec. Add defer GinkgoRecover() at the start of each goroutine that
uses Expect, including the QueryRelationships helper in the singleflight data
reader tests, so assertion failures are handled by Ginkgo instead of crashing or
flaking the suite.
In `@internal/storage/proxies/singleflight/data_reader.go`:
- Around line 33-43: Clone the materialized protobuf results before returning
from the singleflight paths in data_reader.go: the tuples returned by the
QueryRelationships flow and the attribute objects returned by
QuerySingleAttribute and QueryAttributes should be deep-copied after
draining/materializing and before constructing the final iterator or response.
Use the existing helpers around queryRelationshipsGroup and any attribute-query
singleflight logic to ensure each caller gets its own copies, preventing shared
protobuf pointers from being mutated across duplicate requests.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74726ccc-8d17-4eb3-bb5e-08eedb224cb7
π Files selected for processing (2)
internal/storage/proxies/singleflight/data_reader.gointernal/storage/proxies/singleflight/data_reader_test.go
Codecov Reportβ
All modified and coverable lines are covered by tests. β Your project check has failed because the head coverage (74.87%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #3023 +/- ##
==========================================
+ Coverage 74.67% 74.87% +0.20%
==========================================
Files 83 83
Lines 9212 9281 +69
==========================================
+ Hits 6878 6948 +70
Misses 1798 1798
+ Partials 536 535 -1 β View full report in Codecov by Harness. π New features to boost your workflow:
|
So far many similar parallel requests lead to a lot of DB calls fetching the same tuples and competing for DB pool.
This PR uses singleflight for such requests as well to improve the performance.
Summary by CodeRabbit
New Features
Bug Fixes