tillrohrmann commented on a change in pull request #13131:
URL: https://github.com/apache/flink/pull/13131#discussion_r469740064
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##########
@@ -117,13 +117,10 @@ protected void respondAsLeader(ChannelHandlerContext ctx,
RoutedRequest routedRe
FileUploads uploadedFiles = null;
try {
- synchronized (this) {
- if (terminationFuture != null) {
- log.debug("The handler instance for {}
had already been closed.",
untypedResponseMessageHeaders.getTargetRestEndpointURL());
- ctx.channel().close();
- return;
- }
- inFlightRequestTracker.registerRequest();
+ if (!inFlightRequestTracker.registerRequest()) {
Review comment:
No, this works because we terminate the `AbstractHandler` (completing
the `terminationFuture`) once the handler is closed and then all in flight
requests are completed. See this line:
untime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java#L253.
Afterwards the `inFlightRequestTracker` will no longer accept new requests
because it has already terminated. It is effectively the same as
`terminationFuture.isDone()`.
What won't work properly though is if a new request comes which creates a
new asynchronous operation. I think for this to work, the
`CompletedOperationCache` needs to track whether `closeAsync` had been called
or 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.
For queries about this service, please contact Infrastructure at:
[email protected]
