Skip to content

xds: Add a test for incorrect load reporting when using pickfirst with servers in multiple localities#7357

Merged
easwars merged 13 commits into
grpc:masterfrom
arjan-bal:fix-locality-load-reporting
Jun 28, 2024
Merged

xds: Add a test for incorrect load reporting when using pickfirst with servers in multiple localities#7357
easwars merged 13 commits into
grpc:masterfrom
arjan-bal:fix-locality-load-reporting

Conversation

@arjan-bal

@arjan-bal arjan-bal commented Jun 26, 2024

Copy link
Copy Markdown
Contributor

This PR addressed the first task for #7339, adding a unit test that fails. The test presently asserts that the incorrect region received the load until the bug has been fixed:

The test asserts the following after sending 1 rpc:

  1. Server 1 received the rpc
  2. region-2 receives the load (wrong, but represents the current behaviour)

Then we stop server 1, send another rpc and assert the following:

  1. Server 2 received the rpc
  2. region-2 receives the load

Issue: #7339

RELEASE NOTES: None

@arjan-bal arjan-bal changed the title Add a failing test for load reporting when using pickfirst with servers in mulitple localities xds: Add a failing test for load reporting when using pickfirst with servers in mulitple localities Jun 26, 2024
@arjan-bal arjan-bal added this to the 1.66 Release milestone Jun 26, 2024
@codecov

codecov Bot commented Jun 26, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.32%. Comparing base (98e5dee) to head (3f69df5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7357      +/-   ##
==========================================
- Coverage   81.47%   81.32%   -0.16%     
==========================================
  Files         348      348              
  Lines       26761    26755       -6     
==========================================
- Hits        21804    21758      -46     
- Misses       3772     3794      +22     
- Partials     1185     1203      +18     
Files Coverage Δ
internal/stubserver/stubserver.go 84.76% <100.00%> (+0.14%) ⬆️

... and 24 files with indirect coverage changes

@arjan-bal arjan-bal requested a review from easwars June 26, 2024 19:54
@arjan-bal arjan-bal self-assigned this Jun 26, 2024
@arjan-bal arjan-bal requested a review from dfawley June 26, 2024 19:54
@arjan-bal arjan-bal removed their assignment Jun 26, 2024
@arjan-bal arjan-bal force-pushed the fix-locality-load-reporting branch from 33c0345 to 02fa7a4 Compare June 26, 2024 19:57
Comment thread internal/stubserver/stubserver.go
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
@easwars easwars assigned arjan-bal and unassigned easwars Jun 26, 2024
@arjan-bal arjan-bal requested a review from easwars June 27, 2024 07:03
@arjan-bal arjan-bal assigned easwars and unassigned arjan-bal Jun 27, 2024
@arjan-bal arjan-bal force-pushed the fix-locality-load-reporting branch from fea5c9e to 503a123 Compare June 27, 2024 09:30
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
@arjan-bal arjan-bal force-pushed the fix-locality-load-reporting branch from 471c9e3 to b738469 Compare June 27, 2024 10:41
@arjan-bal arjan-bal force-pushed the fix-locality-load-reporting branch from b738469 to c1bdd04 Compare June 27, 2024 10:43

@easwars easwars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great now. Just some very minor nits.

Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread internal/stubserver/stubserver.go
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
@easwars easwars assigned arjan-bal and unassigned easwars Jun 27, 2024
@arjan-bal arjan-bal requested a review from easwars June 27, 2024 18:46
@arjan-bal arjan-bal assigned easwars and unassigned arjan-bal Jun 27, 2024
@arjan-bal arjan-bal changed the title xds: Add a failing test for load reporting when using pickfirst with servers in mulitple localities xds: Add a test for incorrect load reporting when using pickfirst with servers in multiple localities Jun 27, 2024
Comment thread xds/internal/balancer/clusterimpl/tests/balancer_test.go Outdated
@easwars easwars assigned arjan-bal and unassigned easwars Jun 27, 2024
@easwars easwars merged commit f199062 into grpc:master Jun 28, 2024
1 check passed
townba added a commit to townba/grpc-go that referenced this pull request Jul 1, 2024
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 23, 2025
@arjan-bal arjan-bal deleted the fix-locality-load-reporting branch January 15, 2026 12:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants