tillrohrmann edited a comment on pull request #16915:
URL: https://github.com/apache/flink/pull/16915#issuecomment-903729580


   Thanks for the review @zentol.
   
   I think closing the channel in `RespondingChannelHandler` is not valid since 
only the `Client` is allowed to terminate the connection 
(https://github.com/apache/flink/blob/master/flink-queryable-state/flink-queryable-state-client-java/src/main/java/org/apache/flink/queryablestate/network/ClientHandler.java#L112).
 The way you can test the change is by disabling the reusing of established 
connections in the `Client` 
(https://github.com/apache/flink/blob/master/flink-queryable-state/flink-queryable-state-client-java/src/main/java/org/apache/flink/queryablestate/network/Client.java#L354)
 and then remove the `Sharable` annotation. If you then run 
`ClientTest.testConcurrentQueries()` until a failure occurs, then it should 
fail rather soon. PTAL.
   
   It could perfectly be the case that FLINK-23362 solves the problem. If so, I 
still think that this PR is an improvement since it properly defines which 
handler can be reused and which not.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to