Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure the gRPC LoggingInterceptor does not throw exceptions where they cannot be handled #12416

Closed
wants to merge 1 commit into from

Conversation

kevingessner
Copy link
Contributor

This fixes a hang that I've struggled with for ages: when using --build_event_json_file to write a BEP file, enabling --experimental_remote_grpc_log causes the build to hang indefinitely after completion with a message of "Waiting for build events upload: JsonFormatFileTransport".

The default --build_event_json_file_path_conversion ends up doing an RPC call when closing the BEP file. Unfortunately, this call happens after the gRPC log file has already been closed. That exception in the LoggingInterceptor is uncatchable -- it causes the RPC call to hang indefinitely. Therefore, Bazel needs to handle and consume the exception so onClose always succeeds.

@google-cla google-cla bot added the cla: yes label Nov 4, 2020
@coeuvre coeuvre added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Nov 5, 2020
@coeuvre coeuvre self-requested a review November 5, 2020 03:17
Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Hi Kevin, thanks for your catch, pointers and PR!

This also remind me that we maybe handle onClose incorrectly at other places causing #11782.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants