Skip to content

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

Closed
scele wants to merge 1 commit intobazelbuild:masterfrom
scele:issue_12927_part3
Closed

[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs#13106
scele wants to merge 1 commit intobazelbuild:masterfrom
scele:issue_12927_part3

Conversation

@scele
Copy link
Copy Markdown
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

@google-cla google-cla Bot added the cla: yes label Feb 25, 2021
@scele scele mentioned this pull request Feb 25, 2021
Closed
meteorcloudy
meteorcloudy reviewed Feb 25, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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(-)

@scele scele force-pushed the issue_12927_part3 branch from 1840cfc to 33ac15d Compare February 26, 2021 08:01
@meteorcloudy
Copy link
Copy Markdown
Member

meteorcloudy commented Feb 26, 2021

OK, thanks!

@philwo
Copy link
Copy Markdown
Member

philwo commented Feb 26, 2021

FYI @coeuvre

@philwo
Copy link
Copy Markdown
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
Closed
9 tasks
@scele
[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs
77e7661 
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 scele force-pushed the issue_12927_part3 branch from 33ac15d to 77e7661 Compare February 26, 2021 16:01
@scele
Copy link
Copy Markdown
Contributor Author

scele commented Feb 26, 2021

@meteorcloudy @philwo rebased!

@meteorcloudy
Copy link
Copy Markdown
Member

meteorcloudy commented Feb 26, 2021

Thanks, merging this now!

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

meteorcloudy commented Mar 1, 2021

Thank you so much! Now merged!

@scele scele mentioned this pull request Mar 1, 2021
Closed
philwo pushed a commit that referenced this pull request Mar 15, 2021
@scele @philwo
[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs
b588ec7 
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
@scele @philwo
[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs
f420744 
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
@scele @indragiek
[3/3] Bump grpc to 1.33.1 to fix corruption when downloading CAS blobs
0733a8a 
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

Reviewers

@meteorcloudy meteorcloudy meteorcloudy left review comments

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@scele @meteorcloudy @philwo