Skip to content

outlierdetection: cleanup temporary pickfirst health listener attribute #8402

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

Merged
merged 3 commits into from
Jun 19, 2025

Conversation

arjan-bal
Copy link
Contributor

Fixes: #7915

During the implementation of dualstack changes, OutlierDetection needed to handle ejection of subchannels using both the connectivity listener and the new health listener. Now that all Petiole LB policies used in xDS delegate to pick_first, OutlierDetection only needs to support ejection through the health listener. The old code for ejection via the connectivity listener has been cleaned up.

RELEASE NOTES: N/A

@arjan-bal arjan-bal added this to the 1.74 Release milestone Jun 17, 2025
@arjan-bal arjan-bal requested a review from easwars June 17, 2025 14:49
@arjan-bal arjan-bal added the Type: Internal Cleanup Refactors, etc label Jun 17, 2025
@arjan-bal arjan-bal force-pushed the outlier-detection-cleanup branch from e5bc491 to c361f98 Compare June 17, 2025 14:53
@arjan-bal arjan-bal changed the title outlierdetection: Cleanup temporary pickfirst health listener attribute outlierdetection: cleanup temporary pickfirst health listener attribute Jun 17, 2025
Copy link

codecov bot commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.42%. Comparing base (082a927) to head (398a6b0).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8402      +/-   ##
==========================================
+ Coverage   82.28%   82.42%   +0.13%     
==========================================
  Files         413      413              
  Lines       40454    40430      -24     
==========================================
+ Hits        33289    33324      +35     
+ Misses       5795     5748      -47     
+ Partials     1370     1358      -12     
Files with missing lines Coverage Δ
balancer/pickfirst/pickfirstleaf/pickfirstleaf.go 86.49% <ø> (-0.08%) ⬇️
xds/internal/balancer/outlierdetection/balancer.go 88.78% <100.00%> (-0.59%) ⬇️
...ernal/balancer/outlierdetection/subconn_wrapper.go 90.66% <100.00%> (+2.36%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arjan-bal arjan-bal removed the request for review from easwars June 17, 2025 14:59
@arjan-bal arjan-bal requested a review from easwars June 18, 2025 05:13
@arjan-bal arjan-bal assigned arjan-bal and easwars and unassigned arjan-bal Jun 18, 2025
// pick_first and outlier detection will only use the health listener for
// ejection. This hack can then be removed.
func IsManagedByPickfirst(addr resolver.Address) bool {
return addr.BalancerAttributes.Value(managedByPickfirstKeyType{}) != nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that the type managedByPickfirstKeyType is not deleted. Nor is the code to set this attribute from newSCData. Any reason we still need them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, I forgot to clean it up. Removed now.

@easwars easwars assigned arjan-bal and unassigned easwars Jun 18, 2025
@arjan-bal arjan-bal merged commit 0100d21 into grpc:master Jun 19, 2025
16 checks passed
@arjan-bal arjan-bal deleted the outlier-detection-cleanup branch June 19, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

outlierdetection, pickfirst: Remove the balancer attribute to skip ejection using the connectivity state listener
2 participants