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

[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs #13106

Closed
wants to merge 1 commit into from

Conversation

scele
Copy link
Contributor

@scele scele commented Feb 25, 2021

Part 3: remove 1.32.x from third_party/grpc.

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed. In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError. This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

#12927

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Why the jar files are appeared as moved instead of deleted then added?

@scele
Copy link
Contributor Author

scele commented Feb 26, 2021

Why the jar files are appeared as moved instead of deleted then added?

I think it's a github UI glitch. Git shows that the files are changing:

$ git diff HEAD^^ --stat
 third_party/grpc/BUILD                                                            |  18 +++++++++---------
 third_party/grpc/README.bazel.md                                                  |  12 ++++++------
 third_party/grpc/{bazel_1.32.0.patch => bazel_1.33.1.patch}                       |   0
 third_party/grpc/compiler/src/java_plugin/cpp/java_generator.h                    |   4 ++--
 third_party/grpc/{grpc-api-1.32.2.jar => grpc-api-1.33.1.jar}                     | Bin 228400 -> 230504 bytes
 third_party/grpc/{grpc-auth-1.32.2.jar => grpc-auth-1.33.1.jar}                   | Bin 14497 -> 14497 bytes
 third_party/grpc/{grpc-context-1.32.2.jar => grpc-context-1.33.1.jar}             | Bin 30569 -> 30598 bytes
 third_party/grpc/{grpc-core-1.32.2.jar => grpc-core-1.33.1.jar}                   | Bin 634458 -> 646043 bytes
 third_party/grpc/{grpc-netty-1.32.2.jar => grpc-netty-1.33.1.jar}                 | Bin 242645 -> 245937 bytes
 third_party/grpc/{grpc-protobuf-1.32.2.jar => grpc-protobuf-1.33.1.jar}           | Bin 5179 -> 5178 bytes
 third_party/grpc/{grpc-protobuf-lite-1.32.2.jar => grpc-protobuf-lite-1.33.1.jar} | Bin 7629 -> 7628 bytes
 third_party/grpc/{grpc-stub-1.32.2.jar => grpc-stub-1.33.1.jar}                   | Bin 49585 -> 49720 bytes
 third_party/grpc/{grpc_1.32.0.patch => grpc_1.33.1.patch}                         |   0
 13 files changed, 17 insertions(+), 17 deletions(-)

@meteorcloudy
Copy link
Member

OK, thanks!

@philwo
Copy link
Member

philwo commented Feb 26, 2021

FYI @coeuvre

@philwo
Copy link
Member

philwo commented Feb 26, 2021

@scele Thank you for the PR series! I think this would be ready to merge, now that the other two are in.

Could you please rebase this, so that we can get a clean CI run before merging it?

@philwo philwo mentioned this pull request Feb 26, 2021
9 tasks
Part 3: remove 1.32.x from third_party/grpc.

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

bazelbuild#12927
@scele
Copy link
Contributor Author

scele commented Feb 26, 2021

@meteorcloudy @philwo rebased!

@meteorcloudy
Copy link
Member

Thanks, merging this now!

@bazel-io bazel-io closed this in 9b30172 Mar 1, 2021
@meteorcloudy
Copy link
Member

Thank you so much! Now merged!

philwo pushed a commit that referenced this pull request Mar 15, 2021
Part 3: remove 1.32.x from third_party/grpc.

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

#12927

Closes: #13106
philwo pushed a commit that referenced this pull request Mar 15, 2021
Part 3: remove 1.32.x from third_party/grpc.

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

#12927

Closes: #13106
indragiek pushed a commit to specto-dev/bazel that referenced this pull request Mar 30, 2021
Part 3: remove 1.32.x from third_party/grpc.

grpc-java versions 1.27 through 1.32 had a bug where messages could arrive
after the call was reported clsoed.  In the case of bazel, this meant that
in GrpcCacheClient, onNext could be called after onError.  This leads to
offset bookkeeping getting out of sync, and corrupts the CAS blob download.

bazelbuild#12927

Closes: bazelbuild#13106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants