-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Upgrade GRPC to 1.31 to avoid deadlock #8351
Conversation
I opened #8361 since another PR job failed with this kind of exception:
in https://github.com/apache/pulsar/runs/1297863577?check_suite_focus=true I'd assume that protoc-gen-grpc-java.version should match grpc.version . I also added a change in the PR to use the grpc-bom in dependencyManagement. |
This reverts commit 647d3c2.
@pkumar-singh @merlimat I have issued a PR to revert this grpc upgrade:Please see #8363 for more details. The master branch is currently broken because of the grpc upgrade. |
@pkumar-singh regarding the dead lock, did you find some reference elsewhere that GRPC 1.31 has a fix for some dead lock bug? I found an open issue grpc/grpc-java#3084 which is not resolved. There the workaround seems to be to execute callbacks on a different thread (executor). This is the reason why I'm wondering why the upgrade to GRPC 1.31 fixes the issue. It would be useful to understand the full context. |
Well, The context is. I was running bookkeeper table service and I encounter this deadlock. As can be seen from the call stack. org.apache.bookkeeper.statelib.impl.journal.AbstractStateStoreWithJournal.lambda$executeIO$16(AbstractStateStoreWithJournal.java:472). And I saw the grpc/grpc-java#3084 issue too while digging around. How do I confirm upgrade to 1.31 fixes the deadlock. Well, I upgraded to 1.31 in the deployment and deadlock never happened otherwise used to happen within 2 minutes. Besides everything, sooner or later gRPC have to be updated in Apache Pulsar as well. Apache Pulsar running with gRPC 1.18 does have a deadlock. @lhotari |
### Motivation For managing protobuf libraries there is a maven bom file available for protobuf. The benefit of using this is that it can be imported to the project's pom.xml dependency management to make sure that the versions of the various protobuf libraries are aligned and use the same version. Besides starting to use protobuf-bom, it was noticed that there are separate settings for protobuf protoc version (`protoc3.version`) and the protoc grpc plugin versions (`protoc-gen-grpc-java.version`). These versions should match the protobuf and grpc versions. The PR also covers an improvement for that. One motivation of this PR is to prepare for the grpc upgrade that was attempted by #8351 , but reverted. Before doing the grpc upgrade, it would be useful to improve the protobuf & grpc dependency management provided by this PR. ### Modifications * Use protobuf-bom to manage protobuf library versions * make `protoc3.version` match `protobuf3.version` * there should be no reason that these would be different * make grpc's `protoc-gen-grpc-java.version` match `grpc.version` * there should be no reason that there would be different
Co-authored-by: Prashant Kumar <prashantk@splunk.com>
### Motivation For managing protobuf libraries there is a maven bom file available for protobuf. The benefit of using this is that it can be imported to the project's pom.xml dependency management to make sure that the versions of the various protobuf libraries are aligned and use the same version. Besides starting to use protobuf-bom, it was noticed that there are separate settings for protobuf protoc version (`protoc3.version`) and the protoc grpc plugin versions (`protoc-gen-grpc-java.version`). These versions should match the protobuf and grpc versions. The PR also covers an improvement for that. One motivation of this PR is to prepare for the grpc upgrade that was attempted by apache#8351 , but reverted. Before doing the grpc upgrade, it would be useful to improve the protobuf & grpc dependency management provided by this PR. ### Modifications * Use protobuf-bom to manage protobuf library versions * make `protoc3.version` match `protobuf3.version` * there should be no reason that these would be different * make grpc's `protoc-gen-grpc-java.version` match `grpc.version` * there should be no reason that there would be different
Co-authored-by: Prashant Kumar <prashantk@splunk.com>
### Motivation For managing protobuf libraries there is a maven bom file available for protobuf. The benefit of using this is that it can be imported to the project's pom.xml dependency management to make sure that the versions of the various protobuf libraries are aligned and use the same version. Besides starting to use protobuf-bom, it was noticed that there are separate settings for protobuf protoc version (`protoc3.version`) and the protoc grpc plugin versions (`protoc-gen-grpc-java.version`). These versions should match the protobuf and grpc versions. The PR also covers an improvement for that. One motivation of this PR is to prepare for the grpc upgrade that was attempted by apache#8351 , but reverted. Before doing the grpc upgrade, it would be useful to improve the protobuf & grpc dependency management provided by this PR. ### Modifications * Use protobuf-bom to manage protobuf library versions * make `protoc3.version` match `protobuf3.version` * there should be no reason that these would be different * make grpc's `protoc-gen-grpc-java.version` match `grpc.version` * there should be no reason that there would be different
Motivation
Current version of gRPC is deadlocks as explained below.
This deadlock is currently only appearing in state store but might affect pulsar in general as well.
Solution
Verified that with gRPC-1.31 issue is fixed.