Change in asterixdb[master]: [NO ISSUE] Wait for cancels on timeout to prevent DoS

[Michael Blow (Code Review)]

Michael Blow has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/2028

Change subject: [NO ISSUE] Wait for cancels on timeout to prevent DoS
......................................................................

[NO ISSUE] Wait for cancels on timeout to prevent DoS

- Interrupted queries (i.e. node shutdown) will not wait for
cancels to complete before propagating the interrupt

Change-Id: I9f92c65a8adf7bc0dc30e222e396870dda15ea4d
---
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
2 files changed, 13 insertions(+), 10 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/28/2028/1

diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
index 254b6f0..f4c3949 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
@@ -81,8 +81,7 @@
             }
             long timeout = 
ExecuteStatementRequestMessage.DEFAULT_NC_TIMEOUT_MILLIS;
             if (param.timeout != null) {
-                timeout = java.util.concurrent.TimeUnit.NANOSECONDS
-                        
.toMillis(Duration.parseDurationStringToNanos(param.timeout));
+                timeout = 
TimeUnit.NANOSECONDS.toMillis(Duration.parseDurationStringToNanos(param.timeout));
             }
             ExecuteStatementRequestMessage requestMsg =
                     new ExecuteStatementRequestMessage(ncCtx.getNodeId(), 
responseFuture.getFutureId(), queryLanguage,
@@ -91,12 +90,14 @@
             outExecStartEnd[0] = System.nanoTime();
             ncMb.sendMessageToCC(requestMsg);
             try {
-                responseMsg = (ExecuteStatementResponseMessage) 
responseFuture.get(timeout,
-                        java.util.concurrent.TimeUnit.MILLISECONDS);
-            } catch (InterruptedException | TimeoutException exception) {
+                responseMsg = (ExecuteStatementResponseMessage) 
responseFuture.get(timeout, TimeUnit.MILLISECONDS);
+            } catch (InterruptedException e) {
+                cancelQuery(ncMb, ncCtx.getNodeId(), param.clientContextID, e, 
false);
+                throw e;
+            } catch (TimeoutException exception) {
                 RuntimeDataException hde = new 
RuntimeDataException(ErrorCode.QUERY_TIMEOUT, exception);
                 // cancel query
-                cancelQuery(ncMb, ncCtx.getNodeId(), param.clientContextID, 
hde);
+                cancelQuery(ncMb, ncCtx.getNodeId(), param.clientContextID, 
hde, true);
                 throw hde;
             }
             outExecStartEnd[1] = System.nanoTime();
@@ -127,14 +128,16 @@
     }
 
     private void cancelQuery(INCMessageBroker messageBroker, String nodeId, 
String clientContextID,
-            Exception exception) {
+            Exception exception, boolean wait) {
         MessageFuture cancelQueryFuture = 
messageBroker.registerMessageFuture();
         try {
             CancelQueryRequest cancelQueryMessage =
                     new CancelQueryRequest(nodeId, 
cancelQueryFuture.getFutureId(), clientContextID);
             messageBroker.sendMessageToCC(cancelQueryMessage);
-            
cancelQueryFuture.get(ExecuteStatementRequestMessage.DEFAULT_QUERY_CANCELLATION_WAIT_MILLIS,
-                    TimeUnit.MILLISECONDS);
+            if (wait) {
+                
cancelQueryFuture.get(ExecuteStatementRequestMessage.DEFAULT_QUERY_CANCELLATION_WAIT_MILLIS,
+                        TimeUnit.MILLISECONDS);
+            }
         } catch (Exception e) {
             exception.addSuppressed(e);
         } finally {
diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
index 28e55a6..27cdb66 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
@@ -62,7 +62,7 @@
     //TODO: Make configurable: 
https://issues.apache.org/jira/browse/ASTERIXDB-2062
     public static final long DEFAULT_NC_TIMEOUT_MILLIS = 
TimeUnit.MINUTES.toMillis(5);
     //TODO: Make configurable: 
https://issues.apache.org/jira/browse/ASTERIXDB-2063
-    public static final long DEFAULT_QUERY_CANCELLATION_WAIT_MILLIS = 
TimeUnit.MINUTES.toMillis(0);
+    public static final long DEFAULT_QUERY_CANCELLATION_WAIT_MILLIS = 
TimeUnit.MINUTES.toMillis(1);
     private final String requestNodeId;
     private final long requestMessageId;
     private final ILangExtension.Language lang;

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2028
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f92c65a8adf7bc0dc30e222e396870dda15ea4d
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <mb...@apache.org>