client/tso: encapsulate request metadata - #11084
Conversation
Signed-off-by: disksing <i@disksing.com>
Signed-off-by: disksing <i@disksing.com>
ti-chi-bot
Bot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
the
size/XL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTSO request identifiers now use a shared metadata structure. Stream adapters, dispatch logic, queued requests, and tests use the new contract. Legacy service-discovery tests cover successful and failing protocol responses. ChangesTSO metadata propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
🤖 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 `@client/servicediscovery/tso_service_discovery_test.go`:
- Around line 66-68: Update the goroutine invoking grpcServer.Serve to capture
its returned error and handle it explicitly, using the test’s existing failure
or shutdown pattern where appropriate; do not discard the Serve result.
🪄 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 Plus
Run ID: 62e535b2-cfc5-4b4d-b1e0-4162368e9702
📒 Files selected for processing (4)
client/clients/tso/dispatcher.goclient/clients/tso/stream.goclient/clients/tso/stream_test.goclient/servicediscovery/tso_service_discovery_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11084 +/- ##
==========================================
+ Coverage 79.17% 79.38% +0.20%
==========================================
Files 541 541
Lines 76487 76655 +168
==========================================
+ Hits 60558 60849 +291
+ Misses 11629 11541 -88
+ Partials 4300 4265 -35
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Signed-off-by: disksing <i@disksing.com>
|
handled the grpc server serve error in the test as suggested |
|
/retest |
1 similar comment
|
/retest |
|
/test pull-unit-test-next-gen-2 |
There was a problem hiding this comment.
Pull request overview
This PR prepares the client-side TSO path for future API V3 identity expansion by (1) adding characterization tests for existing/legacy request shapes and keyspace-group lookup behavior, and (2) encapsulating the existing cluster/keyspace/keyspace-group identifiers into an internal tsoRequestMetadata value so later identity additions can remain localized.
Changes:
- Add characterization tests for legacy
FindGroupByKeyspaceIDbehavior (requests, responses, error paths, and connection removal on callee mismatch). - Introduce internal
tsoRequestMetadataand thread it through the TSO stream adapter/dispatcher path without changing wire protocol behavior. - Add characterization tests for the PD and standalone TSO stream adapters’ exact request/response conversion.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| client/servicediscovery/tso_service_discovery_test.go | Adds a gRPC test server and characterization tests for legacy FindGroupByKeyspaceID request/response/error handling. |
| client/clients/tso/stream.go | Introduces tsoRequestMetadata and updates stream adapter Send signature to take metadata as a single value. |
| client/clients/tso/stream_test.go | Adds characterization tests for PD/TSO stream adapter request generation and response decoding; updates mocks to use metadata. |
| client/clients/tso/dispatcher.go | Builds and passes tsoRequestMetadata into stream request processing. |
| modRevision, | ||
| ) | ||
|
|
||
| request := <-server.requests |
Signed-off-by: disksing <i@disksing.com>
|
addressed the review: bounded the wait for the FindGroupByKeyspaceID request with a 5s timeout in the characterization test |
|
/test pull-integration-realcluster-test |
2 similar comments
|
/test pull-integration-realcluster-test |
|
/test pull-integration-realcluster-test |
What problem does this PR solve?
Issue Number: ref #10989
The API V3 client work needs to extend the metadata carried by TSO requests. Before adding new identity fields, the existing keyspace-ID path should be covered by characterization tests and represented as one internal value so later changes remain focused and reviewable.
What is changed and how does it work?
This PR:
tsoRequestMetadatavalue for the existing request fields;Check List
Tests
Code changes
Side effects
Related changes
Release note
Summary by CodeRabbit
Bug Fixes
Tests