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:
Yes and no. It could also be the case that we have another in flight
request which simply checks the status of something and does not request a the
result of a completed operation.
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]
