[core] upgrade grpc to v1.58.0#61499
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the gRPC dependency from version 1.57.1 to 1.58.0, and also updates the dependent boringssl library. A new patch, grpc-nextresult-cancelled-init.patch, is introduced to fix an uninitialized variable issue in gRPC's NextResult class. For future contributions, please consider filling out the pull request description to provide context about the changes and their motivation, which greatly helps reviewers.
Note: Security Review did not run due to the size of the PR.
…ect#61449) This reverts commit 544a40f. Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
68d906b to
11dbcc1
Compare
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
11dbcc1 to
3622f49
Compare
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
1ad62c0 to
7924c54
Compare
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
8444de9 to
d2aedf6
Compare
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
| + NextResult() : center_(nullptr), cancelled_(true) {} | ||
| explicit NextResult(RefCountedPtr<pipe_detail::Center<T>> center) | ||
| - : center_(std::move(center)) { | ||
| + : center_(std::move(center)), cancelled_(false) { |
There was a problem hiding this comment.
This makes the ubsan check on the grpc library pass.
44dd2fa to
5185888
Compare
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
5185888 to
5d88dc4
Compare
| std::unordered_map<NodeID, std::shared_ptr<rpc::GrpcServer>> servers; | ||
| std::unordered_set<NodeID> dead_nodes; | ||
| ray::observability::FakeHistogram fake_health_check_rpc_latency_ms_histogram_; | ||
| std::shared_ptr<gcs::GcsHealthCheckManager> health_check; |
There was a problem hiding this comment.
This fix tsan failure. The Async health-check callbacks may still run during teardown and record metric latency. Previously, FakeHistogram could be destroyed before health_check, causing races and failures.
| package( | ||
| default_visibility = ["//:__subpackages__"], | ||
| - features = [ | ||
| - "layering_check", |
There was a problem hiding this comment.
Suppress the undeclared inclusion flood in the build logs. This won't affect runtime behavior. See #61623 for more details.
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
|
@aslonnie @andrew-anyscale to take a look as well |
| sha256 = "ec64fdab22726d50fc056474dd29401d914cc616f53ab8f2fe4866772881d581", | ||
| patches = [ | ||
| "@io_ray//thirdparty/patches:grpc-cython-copts.patch", | ||
| "@io_ray//thirdparty/patches:grpc-disable-layering-check.patch", |
There was a problem hiding this comment.
Can you add a comment referencing bug fix in Bazel 7.3.0 bazelbuild/bazel#21592, as well as llvm/llvm-project@5bba176 LLVM using the same workaround in their BUILD files (llvm/llvm-project@5bba176)
Signed-off-by: Rueian Huang <rueiancsie@gmail.com>
andrew-anyscale
left a comment
There was a problem hiding this comment.
👏 nicely done! Approved
Description
Reintroduce the grpc upgrade for fixing setenv/getenv races. postmerge and premerge, including tsan, ubsan, macos, windows, tests passed.
Related issues
#61195
Additional information