Skip to content

Conversation

@eshitachandwani
Copy link
Member

@eshitachandwani eshitachandwani commented Aug 26, 2025

Original PR: #8483
Related issue: #8474

RELEASE NOTES:

  • lrsclient:
    • Fix a race condition where the LRSClient was not initialized at
      creation time but it was being initialized at the time of calling the
      ReportLoad function.
    • Creating an LRSClient no longer requires a node ID.

@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.54%. Comparing base (7269d5f) to head (120046e).
⚠️ Report is 2 commits behind head on v1.75.x.

Files with missing lines Patch % Lines
xds/internal/xdsclient/clientimpl.go 80.00% 2 Missing and 1 partial ⚠️
xds/internal/clients/lrsclient/lrsclient.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           v1.75.x    #8541      +/-   ##
===========================================
- Coverage    81.77%   81.54%   -0.24%     
===========================================
  Files          413      413              
  Lines        40530    40623      +93     
===========================================
- Hits         33145    33126      -19     
- Misses        6003     6029      +26     
- Partials      1382     1468      +86     
Files with missing lines Coverage Δ
xds/internal/xdsclient/clientimpl_loadreport.go 36.00% <ø> (-36.73%) ⬇️
xds/internal/clients/lrsclient/lrsclient.go 44.24% <0.00%> (-28.39%) ⬇️
xds/internal/xdsclient/clientimpl.go 60.74% <80.00%> (-21.79%) ⬇️

... and 24 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
Copy link
Contributor

Why do we need to cherry-pick #8467? It's a fix for a flaky test, right?

@eshitachandwani
Copy link
Member Author

Why do we need to cherry-pick #8467? It's a fix for a flaky test, right?

Yes, but it is a new test introduced in v1.75 (#8428) , so I thought it would make sense to put the fix in v1.75 as well.
WDYT?

@arjan-bal
Copy link
Contributor

We don't need to cherry-pick fixes for flaky tests.

Fixes: grpc#8474

The race is in
[ReportLoad](https://github.com/grpc/grpc-go/blob/9186ebd774370e3b3232d1b202914ff8fc2c56d6/xds/internal/xdsclient/clientimpl_loadreport.go#L35C2-L44C21)
function of clientImpl. The implementation was recently changed as the
part of [xds client
migration](grpc@082a927).

The
[comment](https://github.com/grpc/grpc-go/blob/85240a5b02defe7b653ccba66866b4370c982b6a/xds/internal/xdsclient/clientimpl.go#L86C2-L87C16)
says that `lrsclient.LRSClient` should be initialized only at creation
time but that was not the case. It was being initialized at the time of
calling `ReportLoad` function.

RELEASE NOTES:

- lrsclient:
- Fix a race condition where the `LRSClient` was not initialized at
creation time but it was being initialized at the time of calling the
`ReportLoad` function.
	- 	Creating an `LRSClient` no longer requires a node ID.
@eshitachandwani eshitachandwani changed the title Cherry pick #8483 and #8467 into v1.75.x Cherry pick #8483 into v1.75.x Aug 27, 2025
@easwars
Copy link
Contributor

easwars commented Aug 27, 2025

We don't need to cherry-pick fixes for flaky tests.

+1

@easwars easwars assigned eshitachandwani and unassigned easwars and arjan-bal Aug 27, 2025
@eshitachandwani eshitachandwani merged commit a52e42b into grpc:v1.75.x Aug 28, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants