>From Murtadha Hubail <[email protected]>:

Murtadha Hubail has submitted this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20496?usp=email )

Change subject: [NO ISSUE][RT] Respond to NC in case of unexpected failures
......................................................................

[NO ISSUE][RT] Respond to NC in case of unexpected failures

- user model changes: no
- storage format changes: no
- interface changes: yes

Details:

- On unexpected failures while sending messages from CC to NC,
  send a failure response to the NC to avoid requests hanging
  on the NC until their timeout.

Ext-ref: MB-68985
Change-Id: I5ff53205d7cbb23c91a482b22165e1645247745f
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20496
Reviewed-by: Murtadha Hubail <[email protected]>
Integration-Tests: Jenkins <[email protected]>
Reviewed-by: Hussain Towaileb <[email protected]>
Tested-by: Murtadha Hubail <[email protected]>
---
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/AbstractInternalRequestMessage.java
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/CCMessageBroker.java
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/messaging/api/ICCMessageBroker.java
4 files changed, 40 insertions(+), 11 deletions(-)

Approvals:
  Murtadha Hubail: Looks good to me, but someone else must approve; Verified
  Jenkins: Verified
  Hussain Towaileb: Looks good to me, approved




diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/AbstractInternalRequestMessage.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/AbstractInternalRequestMessage.java
index 7d58e4e..41c4906 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/AbstractInternalRequestMessage.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/AbstractInternalRequestMessage.java
@@ -113,6 +113,8 @@
             messageBroker.sendApplicationMessageToNC(responseMsg, 
nodeRequestId);
         } catch (Exception e) {
             LOGGER.log(Level.WARN, e.toString(), e);
+            responseMsg.setError(new Exception(e.getMessage()));
+            messageBroker.sendMessageQuietly(responseMsg, nodeRequestId);
         }

     }
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 0cdbf95..94d6309 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
@@ -143,7 +143,7 @@
         CCMessageBroker messageBroker = (CCMessageBroker) 
ccSrvContext.getMessageBroker();
         final RuntimeDataException rejectionReason = getRejectionReason(ccSrv, 
requestNodeId);
         if (rejectionReason != null) {
-            sendRejection(rejectionReason, messageBroker, requestMessageId, 
requestNodeId);
+            sendRejection(rejectionReason, messageBroker);
             return;
         }
         CCExtensionManager ccExtMgr = (CCExtensionManager) 
ccAppCtx.getExtensionManager();
@@ -195,11 +195,7 @@
             GlobalConfig.ASTERIX_LOGGER.log(Level.ERROR, "Unexpected 
exception", e);
             responseMsg.setError(e);
         }
-        try {
-            messageBroker.sendApplicationMessageToNC(responseMsg, 
requestNodeId);
-        } catch (Exception e) {
-            LOGGER.log(Level.WARN, e.toString(), e);
-        }
+        sendResponseToNc(messageBroker, responseMsg);
     }

     protected IRequestParameters 
createRequestParameters(IStatementExecutor.StatementProperties 
statementProperties,
@@ -231,11 +227,8 @@
         return null;
     }

-    protected void sendRejection(Exception reason, CCMessageBroker 
messageBroker, long requestMessageId,
-            String requestNodeId) {
-        ExecuteStatementResponseMessage responseMsg =
-                new ExecuteStatementResponseMessage(requestMessageId, 
clientContextID, requestReference.getUuid());
-        responseMsg.setError(reason);
+    protected void sendRejection(Exception reason, CCMessageBroker 
messageBroker) {
+        ExecuteStatementResponseMessage responseMsg = 
createFailureMessage(reason);
         try {
             messageBroker.sendApplicationMessageToNC(responseMsg, 
requestNodeId);
         } catch (Exception e) {
@@ -243,6 +236,23 @@
         }
     }

+    private void sendResponseToNc(CCMessageBroker messageBroker, 
ExecuteStatementResponseMessage responseMsg) {
+        try {
+            messageBroker.sendApplicationMessageToNC(responseMsg, 
requestNodeId);
+        } catch (Exception e) {
+            LOGGER.error("unexpected exception sending message to node {}", 
requestNodeId, e);
+            ExecuteStatementResponseMessage failureMessage = 
createFailureMessage(new Exception(e.getMessage()));
+            messageBroker.sendMessageQuietly(failureMessage, requestNodeId);
+        }
+    }
+
+    private ExecuteStatementResponseMessage createFailureMessage(Exception 
reason) {
+        ExecuteStatementResponseMessage responseMsg =
+                new ExecuteStatementResponseMessage(requestMessageId, 
clientContextID, requestReference.getUuid());
+        responseMsg.setError(reason);
+        return responseMsg;
+    }
+
     @Override
     public String toString() {
         if (statementsText != null && (statementsText.startsWith("UPSERT") || 
statementsText.startsWith("INSERT"))) {
diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/CCMessageBroker.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/CCMessageBroker.java
index 0683207..eb538f0 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/CCMessageBroker.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/CCMessageBroker.java
@@ -70,6 +70,15 @@
     }

     @Override
+    public void sendMessageQuietly(INcAddressedMessage msg, String nodeId) {
+        try {
+            sendMessage(msg, nodeId, false);
+        } catch (Exception e) {
+            LOGGER.warn("failed to send message to node {}", nodeId, e);
+        }
+    }
+
+    @Override
     public boolean sendRealTimeApplicationMessageToNC(INcAddressedMessage msg, 
String nodeId) throws Exception {
         return sendMessage(msg, nodeId, true);
     }
diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/messaging/api/ICCMessageBroker.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/messaging/api/ICCMessageBroker.java
index e628bc7..72b0040 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/messaging/api/ICCMessageBroker.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/messaging/api/ICCMessageBroker.java
@@ -39,6 +39,14 @@
     boolean sendApplicationMessageToNC(INcAddressedMessage msg, String nodeId) 
throws Exception;

     /**
+     * Attempts to send {@code msg} to the specified {@code nodeId} without 
throwing exceptions
+     *
+     * @param msg
+     * @param nodeId
+     */
+    void sendMessageQuietly(INcAddressedMessage msg, String nodeId);
+
+    /**
      * Sends the passed message to the specified {@code nodeId}
      *
      * @param msg

--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20496?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://asterix-gerrit.ics.uci.edu/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: asterixdb
Gerrit-Branch: phoenix
Gerrit-Change-Id: I5ff53205d7cbb23c91a482b22165e1645247745f
Gerrit-Change-Number: 20496
Gerrit-PatchSet: 4
Gerrit-Owner: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Hussain Towaileb <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>

Reply via email to