[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r406149904
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -221,7 +227,12 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
@Override
public final CompletableFuture closeAsync() {
- return FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ if (isHandlerClosed.compareAndSet(false, true)) {
+ return
FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ } else {
+ log.warn("The handler instance for {} had already been
closed, but another attempt at closing it was made.",
untypedResponseMessageHeaders.getTargetRestEndpointURL());
+ return CompletableFuture.completedFuture(null);
Review comment:
Hi @TisonKun , I have submitted a new commit which makes calls to closeAsync
return the same result despite of multiple calls (idempotent).
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r405287917
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -221,7 +227,12 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
@Override
public final CompletableFuture closeAsync() {
- return FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ if (isHandlerClosed.compareAndSet(false, true)) {
+ return
FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ } else {
+ log.warn("The handler instance for {} had already been
closed, but another attempt at closing it was made.",
untypedResponseMessageHeaders.getTargetRestEndpointURL());
+ return CompletableFuture.completedFuture(null);
Review comment:
> I am not sure whether we need to add such logics here. Since the
`closeAsync` is not designed to be called multiple times and also currently we
do not have this behavior. We close the handler one by one in
`RestEndpoint#closeHandlersAsync`.
Hi @wangyang0918 , I believe that this code is to prevent future developer
from accidentally reusing the same handlers multiple times. As you can see, the
bug described in this JIRA ticket was caused by inappropriate reuse of one REST
handler.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404762229
##
File path:
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNJobCancellationITCase.java
##
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.yarn;
+
+import org.apache.flink.client.deployment.ClusterSpecification;
+import org.apache.flink.client.program.ClusterClient;
+import org.apache.flink.configuration.AkkaOptions;
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.configuration.MemorySize;
+import org.apache.flink.configuration.TaskManagerOptions;
+import org.apache.flink.runtime.jobgraph.JobGraph;
+import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
+import org.apache.flink.streaming.api.functions.sink.DiscardingSink;
+import org.apache.flink.streaming.api.functions.source.SourceFunction;
+import org.apache.flink.yarn.util.YarnTestUtils;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.CompletableFuture;
+
+
+/**
+ * Test cases for the cancellation of Yarn Flink clusters.
+ */
+public class YARNJobCancellationITCase extends YarnTestBase {
Review comment:
> Though we don't require the test for this PR, it is worth pulled in in
another commit. An end-to-end test of cancel command in YARN per-job mode
guards failures caused by other changes in future developing.
Maybe we could have more integrated test cases on the execution flow, like
cancel(), stopWithSavepoint(), etc., to uncover more issues that possibly slip
through the unit test cases.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404755577
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -221,7 +227,12 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
@Override
public final CompletableFuture closeAsync() {
- return FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ if (isHandlerClosed.compareAndSet(false, true)) {
+ return
FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ } else {
+ log.warn("The handler instance for {} had already been
closed, but another attempt at closing it was made.",
untypedResponseMessageHeaders.getTargetRestEndpointURL());
+ return CompletableFuture.completedFuture(null);
Review comment:
> I don't think we should return a completed future if the handle has
received "close" message. It is possible that the handle is still closing.
Possibly we alter `inFlightRequestTracker::awaitAsync` to return the terminate
future if it is closed, without deregister the phaser party.
>
> cc @zentol @GJL
Hi @TisonKun , thanks for your review.
My original thought was that a completed ComplatableFuture
*CompletableFuture.completedFuture(null)* would indicate that this invocation
has finished without any longer effect, as it has nothing to do with the
previous call from the user's perspective.
To my understanding, if the REST handler was previously requested to be
closed, then the corresponding CompletableFuture would have already been sent
to the user, and if that CompletedFuture is still not finished,
org.apache.flink.runtime.rest.RestServerEndpoint#closeHandlersAsync would wait
for its completion, so it would not cause the RestServerEndpoint to shutdown
earlier.
Or else maybe we could just throw an *Exception* if the handler is being
closed more than once? This would make the design simpler.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404755577
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -221,7 +227,12 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
@Override
public final CompletableFuture closeAsync() {
- return FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ if (isHandlerClosed.compareAndSet(false, true)) {
+ return
FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ } else {
+ log.warn("The handler instance for {} had already been
closed, but another attempt at closing it was made.",
untypedResponseMessageHeaders.getTargetRestEndpointURL());
+ return CompletableFuture.completedFuture(null);
Review comment:
> I don't think we should return a completed future if the handle has
received "close" message. It is possible that the handle is still closing.
Possibly we alter `inFlightRequestTracker::awaitAsync` to return the terminate
future if it is closed, without deregister the phaser party.
>
> cc @zentol @GJL
Hi @TisonKun , thanks for your review.
My original thought was that a completed ComplatableFuture
*CompletableFuture.completedFuture(null)* would indicate that this invocation
has finished without any longer effect, as it has nothing to do with the
previous call.
To my understanding, if the REST handler was previously requested to be
closed, then the corresponding CompletableFuture would have already been sent
to the user, and if that CompletedFuture is still not finished,
org.apache.flink.runtime.rest.RestServerEndpoint#closeHandlersAsync would wait
for its completion, so it would not cause the RestServerEndpoint to shutdown
earlier.
Or else maybe we could just throw an *Exception* if the handler is being
closed more than once? This would make the design simpler.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404706851
##
File path:
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNJobCancellationITCase.java
##
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.yarn;
+
+import org.apache.flink.client.deployment.ClusterSpecification;
+import org.apache.flink.client.program.ClusterClient;
+import org.apache.flink.configuration.AkkaOptions;
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.configuration.MemorySize;
+import org.apache.flink.configuration.TaskManagerOptions;
+import org.apache.flink.runtime.jobgraph.JobGraph;
+import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
+import org.apache.flink.streaming.api.functions.sink.DiscardingSink;
+import org.apache.flink.streaming.api.functions.source.SourceFunction;
+import org.apache.flink.yarn.util.YarnTestUtils;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.CompletableFuture;
+
+
+/**
+ * Test cases for the cancellation of Yarn Flink clusters.
+ */
+public class YARNJobCancellationITCase extends YarnTestBase {
Review comment:
Removed this test as it is not needed now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404706689
##
File path:
flink-clients/src/test/java/org/apache/flink/client/program/rest/RestClusterClientTest.java
##
@@ -271,6 +273,24 @@ public void testDetachedJobSubmission() throws Exception {
}
+ /**
+* Tests that ensure each handler is registered only once.
+*/
+ @Test
+ public void testDuplicatedHandlerRegistration() throws Exception {
+ final TestJobSubmitHandler testJobSubmitHandler = new
TestJobSubmitHandler();
+
+ assertThrows("Duplicate REST handler instance found. Please
ensure each instance is registered only once.",
+ FlinkRuntimeException.class,
+ () -> {
+ try (TestRestServerEndpoint restServerEndpoint
= createRestServerEndpoint(
Review comment:
Hi, I have changed the code to make it more robust.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404697396
##
File path:
flink-clients/src/test/java/org/apache/flink/client/program/rest/RestClusterClientTest.java
##
@@ -271,6 +273,24 @@ public void testDetachedJobSubmission() throws Exception {
}
+ /**
+* Tests that ensure each handler is registered only once.
+*/
+ @Test
Review comment:
Ah sorry it is my fault. Now moved the test case to
RestServerEndpointITCase, which is the definitely right place.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404695754
##
File path:
flink-clients/src/test/java/org/apache/flink/client/program/rest/RestClusterClientTest.java
##
@@ -271,6 +273,24 @@ public void testDetachedJobSubmission() throws Exception {
}
+ /**
+* Tests that ensure each handler is registered only once.
+*/
+ @Test
+ public void testDuplicatedHandlerRegistration() throws Exception {
+ final TestJobSubmitHandler testJobSubmitHandler = new
TestJobSubmitHandler();
+
+ assertThrows("Duplicate REST handler instance found. Please
ensure each instance is registered only once.",
+ FlinkRuntimeException.class,
+ () -> {
+ try (TestRestServerEndpoint restServerEndpoint
= createRestServerEndpoint(
+ testJobSubmitHandler,
testJobSubmitHandler)) {
+
createRestClusterClient(restServerEndpoint.getServerAddress().getPort());
Review comment:
Yes, test cases should only contain necessary code, so I have removed the
useless parts.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404695021
##
File path:
flink-clients/src/test/java/org/apache/flink/client/program/rest/RestClusterClientTest.java
##
@@ -271,6 +273,24 @@ public void testDetachedJobSubmission() throws Exception {
}
+ /**
+* Tests that ensure each handler is registered only once.
+*/
+ @Test
+ public void testDuplicatedHandlerRegistration() throws Exception {
+ final TestJobSubmitHandler testJobSubmitHandler = new
TestJobSubmitHandler();
+
+ assertThrows("Duplicate REST handler instance found. Please
ensure each instance is registered only once.",
Review comment:
Thanks for pointing out this, I have made the matching text shorter as
commented above.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404693049
##
File path:
flink-clients/src/test/java/org/apache/flink/client/program/rest/RestClusterClientTest.java
##
@@ -271,6 +273,24 @@ public void testDetachedJobSubmission() throws Exception {
}
+ /**
+* Tests that ensure each handler is registered only once.
+*/
+ @Test
+ public void testDuplicatedHandlerRegistration() throws Exception {
Review comment:
Hi zentol, I have changed the method name to the recommended one, which is
indeed more clear and intuitive.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404518850
##
File path:
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNJobCancellationITCase.java
##
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.yarn;
+
+import org.apache.flink.client.deployment.ClusterSpecification;
+import org.apache.flink.client.program.ClusterClient;
+import org.apache.flink.configuration.AkkaOptions;
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.configuration.MemorySize;
+import org.apache.flink.configuration.TaskManagerOptions;
+import org.apache.flink.runtime.jobgraph.JobGraph;
+import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
+import org.apache.flink.streaming.api.functions.sink.DiscardingSink;
+import org.apache.flink.streaming.api.functions.source.SourceFunction;
+import org.apache.flink.yarn.util.YarnTestUtils;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.api.records.ApplicationId;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.concurrent.CompletableFuture;
+
+
+/**
+ * Test cases for the cancellation of Yarn Flink clusters.
+ */
+public class YARNJobCancellationITCase extends YarnTestBase {
Review comment:
Thank you for the advice, and I agree that runtime verification is necessary
to prevent users from accidentally reusing the handler.
I have introduced a handler duplication check at *start()* method of
RestServerEndpoint class, and added a test case in the cluster test module.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once URL: https://github.com/apache/flink/pull/11639#discussion_r404517543 ## File path: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/WebMonitorEndpoint.java ## @@ -500,6 +500,15 @@ public WebMonitorEndpoint( JobCancellationHeaders.getInstance(), TerminationModeQueryParameter.TerminationMode.CANCEL); + // this is to prevent the same JobCancellationHandler from being registered twice + // should be removed once the Yarn proxy can forward all REST verbs + final JobCancellationHandler legacyJobCancelTerminationHandler = new JobCancellationHandler( Review comment: You are right, yarnJobCancelTerminationHandler is more clear to indicate that this handler is only designed for dealing with special cases in YARN. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404517387
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -217,11 +223,17 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
HttpResponseStatus.INTERNAL_SERVER_ERROR,
responseHeaders);
}
+
}
@Override
public final CompletableFuture closeAsync() {
- return FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ if (isHandlerClosed.compareAndSet(false, true)) {
+ return
FutureUtils.composeAfterwards(closeHandlerAsync(),
inFlightRequestTracker::awaitAsync);
+ } else {
+ log.warn("Handler instance {} had already been closed,
not allowed to close it again.", this);
Review comment:
Thanks for pointing out this, and I have changed this log message to the
recommended way.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime] Prevent REST handler from being closed more than once
kylemeow commented on a change in pull request #11639: [FLINK-16626][runtime]
Prevent REST handler from being closed more than once
URL: https://github.com/apache/flink/pull/11639#discussion_r404517252
##
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractHandler.java
##
@@ -217,11 +223,17 @@ private void finalizeRequestProcessing(FileUploads
uploadedFiles) {
HttpResponseStatus.INTERNAL_SERVER_ERROR,
responseHeaders);
}
+
Review comment:
Hi zentol, I have removed this line in the new commit : )
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:
us...@infra.apache.org
With regards,
Apache Git Services
