Repository navigation
[build] Use Arena::Create and pin Java 8 bytecode ahead of protobuf 29 / rules_java 8 - #67720
elliot-barn wants to merge 2 commits into
Conversation
Arena::CreateMessage is deprecated in newer protobuf and compiled out under PROTOBUF_FUTURE_REMOVE_CREATEMESSAGE. Arena::Create does the same thing for arena-enabled messages and is available in the protobuf Ray pins today, so this can land ahead of the protobuf 29 bump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
rules_java 7.x compiles Java at source/target 8 by default; 8.x defaults to 11. On the protobuf 29 / rules_java 8 branch (#66483) Ray's jars came out as class file version 55, which a Java 8 runtime rejects (UnsupportedClassVersionError in test_raydp and test_logging_java). Pin --java_language_version=8 now; under rules_java 7.x this matches the default and changes nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
46b6ffc to
239b18b
Compare
elliot-barn
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures Bazel to emit Java 8 bytecode to prevent breaking consumers on a Java 8 runtime. Additionally, it updates Protobuf Arena allocation calls from google::protobuf::Arena::CreateMessage to google::protobuf::Arena::Create in both gcs_actor_manager_test.cc and server_call.h. There are no review comments, so I have no feedback to provide.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 239b18b. Configure here.
| record_metrics_(record_metrics), | ||
| server_metrics_(server_metrics) { | ||
| reply_ = google::protobuf::Arena::CreateMessage<Reply>(&arena_); | ||
| reply_ = google::protobuf::Arena::Create<Reply>(&arena_); |
There was a problem hiding this comment.
Arena Create leaves reply unbound
High Severity
On the current protobuf 23.4 pin, Arena::Create default-constructs the reply and does not pass the arena, so GetArena() stays null. HandleGetAllActorInfo then check-fails when show_dead_jobs is true, which is the path that lists actors from finished jobs.
Reviewed by Cursor Bugbot for commit 239b18b. Configure here.
Sparks0219
left a comment
Sparks0219
left a comment
There was a problem hiding this comment.
lgtm, cursor comment is right but if proto upgrade goes in right after then should be fine
Description
Two forward-compatible changes needed by the protobuf 29 / rules_java 8 bump. Both are no-ops on master's current versions:
Arena::CreateMessagewithArena::Create(server_call.hplus 7 test call sites).CreateMessageis deprecated in newer protobuf. [build] Bazel 8.8.1: bump protobuf to v29.6, rules_java to 8.12.0 and gRPC to 1.69.0 #66483 compiles it out withPROTOBUF_FUTURE_REMOVE_CREATEMESSAGEbecause GCC 10 warns on its body under-Werror.--java_language_version=8. rules_java 7.x already defaults to Java 8. 8.x defaults to 11, which breaks consumers on a Java 8 runtime (e.g. RayDP).Related issues
Split out of #66483 (protobuf 29 / gRPC 1.69) to make that PR easier to review.
🤖 Generated with Claude Code