-
Notifications
You must be signed in to change notification settings - Fork 642
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
[ISSUE #4643]Fix Grpc client can't reconnection when runtime after crashing the runtime and restarting it #4644
Conversation
…ter crashing the runtime and restarting it.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4644 +/- ##
=========================================
Coverage 17.38% 17.39%
Complexity 1757 1757
=========================================
Files 797 797
Lines 29776 29774 -2
Branches 2573 2573
=========================================
+ Hits 5177 5178 +1
+ Misses 24121 24118 -3
Partials 478 478 ☔ View full report in Codecov by Sentry. |
@@ -85,7 +85,8 @@ public EventMeshGrpcConsumer(final EventMeshGrpcClientConfig clientConfig) { | |||
} | |||
|
|||
public void init() { | |||
this.channel = ManagedChannelBuilder.forAddress(clientConfig.getServerAddr(), clientConfig.getServerPort()).usePlaintext().build(); | |||
this.channel = ManagedChannelBuilder.forAddress(clientConfig.getServerAddr(), clientConfig.getServerPort()).enableRetry().usePlaintext() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't gRPC generally supports retry by default? Or rather, it's not like that, we must manually enable retry?
gRPC一般不是默认支持重试吗?还是说并非如此,必须手动开启重试?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't gRPC generally supports retry by default? Or rather, it's not like that, we must manually enable retry?
gRPC一般不是默认支持重试吗?还是说并非如此,必须手动开启重试?
This can be deleted.
@@ -99,8 +99,7 @@ public void onNext(final CloudEvent message) { | |||
|
|||
@Override | |||
public void onError(final Throwable t) { | |||
LogUtils.error(log, "Received Server side error", t); | |||
close(); | |||
log.error("Received Server side error", t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the close() method, will it affect the acceptance of messages when the completed method cannot be called in the end under abnormal circumstances?
您移除了close()方法,那异常情况下最终调用不到completed方法,会不会影响消息的接受?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the close() method, will it affect the acceptance of messages when the completed method cannot be called in the end under abnormal circumstances?
您移除了close()方法,那异常情况下最终调用不到completed方法,会不会影响消息的接受?
My understanding here is that it is not necessary to close it in the case of a stream. Once closed, it will directly disconnect from the server. The SDK will directly exit.
Here is the gRPC description of StreamObserver#onCompleted.
This is also the reason why retries are not possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After StreamObserver#onCompleted is called, the connection should not be disconnected, and the client can continue to receive responses from the server. But this source code comment reminded me that although the connection was not disconnected, gRPC indicates that the client is no longer sending messages to the server. (@hhuang1231 This can be seen as a supplement to this reply #4587 (comment), which may be the reason why heartbeat cannot be sent.)
StreamObserver#onCompleted被调用以后,应该并不会断开连接,客户端可以继续接收服务端的响应。但是这段源码注释提醒了我,虽然连接没断开,但是gRPC这时标识客户端不再发送消息给服务端。(@hhuang1231 这可以看作是对这条回复#4587 (comment) 的补充,这是我认为的可能导致无法发送心跳的原因。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gRPC keepAlive connections have the ability to automatically close the connection when C/S times out, without the need to manually call close().
public void init() { | ||
this.channel = ManagedChannelBuilder.forAddress(clientConfig.getServerAddr(), clientConfig.getServerPort()).usePlaintext().build(); | ||
this.channel = ManagedChannelBuilder.forAddress(clientConfig.getServerAddr(), clientConfig.getServerPort()).usePlaintext() | ||
.build(); | ||
this.consumerClient = ConsumerServiceGrpc.newBlockingStub(channel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The additional line break here is not that good~
Fixes #4643
Motivation
Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.
Modifications
Describe the modifications you've done.
Documentation