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 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

3 participants