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

gracefulswitch: add ParseConfig and make UpdateClientConnState call SwitchTo if needed #7035

Merged
merged 6 commits into from
Mar 19, 2024

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Mar 12, 2024

cc @anicr7

This change does not migrate some users of gracefulswitch; they can be done as a follow-up: clusterimpl, outlierdetection, balancergroup.

RELEASE NOTES: none

@dfawley dfawley added the Type: Feature New features or improvements in behavior label Mar 12, 2024
@dfawley dfawley added this to the 1.63 Release milestone Mar 12, 2024
@dfawley dfawley requested a review from arvindbr8 March 12, 2024 23:39
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

Merging #7035 (d3d4605) into master (dadbbfa) will decrease coverage by 0.14%.
The diff coverage is 84.70%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7035      +/-   ##
==========================================
- Coverage   82.45%   82.31%   -0.14%     
==========================================
  Files         299      300       +1     
  Lines       31314    31349      +35     
==========================================
- Hits        25819    25805      -14     
- Misses       4435     4471      +36     
- Partials     1060     1073      +13     
Files Coverage Δ
balancer/balancer.go 96.00% <100.00%> (+0.16%) ⬆️
balancer_wrapper.go 83.51% <100.00%> (+0.58%) ⬆️
clientconn.go 90.76% <100.00%> (-2.00%) ⬇️
pickfirst.go 82.22% <100.00%> (-0.52%) ⬇️
service_config.go 79.04% <84.21%> (+0.09%) ⬆️
internal/balancer/gracefulswitch/gracefulswitch.go 88.79% <82.60%> (-0.88%) ⬇️
internal/balancer/gracefulswitch/config.go 80.00% <80.00%> (ø)

... and 13 files with indirect coverage changes

@dfawley dfawley added Type: Internal Cleanup Refactors, etc and removed Type: Feature New features or improvements in behavior labels Mar 13, 2024
@zasweq
Copy link
Contributor

zasweq commented Mar 19, 2024

This change LGTM, what was the technical reason this came up (other than the fact this is now how Java and C does it)? Will let Arvind gate the merge though since he's official reviewer.

Copy link
Member Author

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Comments addressed

This change LGTM, what was the technical reason this came up (other than the fact this is now how Java and C does it)? Will let Arvind gate the merge though since he's official reviewer.

The reason we did this is that we had a user that wanted to delegate to a child using GSB but didn't want to have to do all the parsing logic.

@arvindbr8 arvindbr8 assigned dfawley and unassigned arvindbr8 Mar 19, 2024
@dfawley dfawley merged commit faf9964 into grpc:master Mar 19, 2024
14 checks passed
@dfawley dfawley deleted the gsb2 branch March 19, 2024 18:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants