This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 897c96c  Add error message to broker response in case of broker send 
error (#5705)
897c96c is described below

commit 897c96cf45d46ed664883a7c54790454266916f1
Author: Sajjad <[email protected]>
AuthorDate: Mon Jul 20 22:34:00 2020 -0700

    Add error message to broker response in case of broker send error (#5705)
    
    On Pinot Broker, the incoming request gets written into socket channels of 
the target Servers. This happens on `QueryRouter.submitQuery(...)` function. If 
any exception occurs during  submitQuery for any reason like connection refused 
to one of the servers, sending requests to the remaining servers are abandoned 
and the partial responses from already successful sent requests are returned in 
BrokerResponse with no indication of the exception.
    Although partial response is acceptable, there should be an indication of 
such problem in broker response to make life easier for ppl debugging this 
issue. Recently there was an incident where, for a specific use case, broker 
response was empty (no partial response) and there was no exception returned, 
while data was available on Pinot Servers. After good amount of time going 
through logs, this issue was discovered with exception being connection refused 
by only one faulty server.
    This PR adds the exception to BrokerReponse for easier debugging.
---
 .../SingleConnectionBrokerRequestHandler.java        |  8 ++++++++
 .../pinot/common/exception/QueryException.java       | 20 +++++++++++---------
 .../pinot/core/transport/AsyncQueryResponse.java     | 10 ++++++++++
 .../org/apache/pinot/core/transport/QueryRouter.java |  1 +
 4 files changed, 30 insertions(+), 9 deletions(-)

diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
index 862dae1..420177a 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
@@ -31,12 +31,14 @@ import org.apache.pinot.broker.api.RequestStatistics;
 import org.apache.pinot.broker.broker.AccessControlFactory;
 import org.apache.pinot.broker.queryquota.QueryQuotaManager;
 import org.apache.pinot.broker.routing.RoutingManager;
+import org.apache.pinot.common.exception.QueryException;
 import org.apache.pinot.common.metrics.BrokerMeter;
 import org.apache.pinot.common.metrics.BrokerMetrics;
 import org.apache.pinot.common.metrics.BrokerQueryPhase;
 import org.apache.pinot.common.request.BrokerRequest;
 import org.apache.pinot.common.response.BrokerResponse;
 import org.apache.pinot.common.response.broker.BrokerResponseNative;
+import org.apache.pinot.common.response.broker.QueryProcessingException;
 import org.apache.pinot.common.utils.DataTable;
 import org.apache.pinot.common.utils.HashUtil;
 import org.apache.pinot.core.transport.AsyncQueryResponse;
@@ -114,6 +116,12 @@ public class SingleConnectionBrokerRequestHandler extends 
BaseBrokerRequestHandl
     brokerResponse.setNumServersQueried(numServersQueried);
     brokerResponse.setNumServersResponded(numServersResponded);
 
+    Exception brokerRequestSendException = 
asyncQueryResponse.getBrokerRequestSendException();
+    if (brokerRequestSendException != null) {
+      String errorMsg = 
QueryException.getTruncatedStackTrace(brokerRequestSendException);
+      brokerResponse
+          .addToExceptions(new 
QueryProcessingException(QueryException.BROKER_REQUEST_SEND_ERROR_CODE, 
errorMsg));
+    }
     if (brokerResponse.getExceptionsSize() > 0) {
       _brokerMetrics.addMeteredTableValue(rawTableName, 
BrokerMeter.BROKER_RESPONSES_WITH_PROCESSING_EXCEPTIONS, 1);
     }
diff --git 
a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
 
b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
index b0b9be5..89dda00 100644
--- 
a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
+++ 
b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
@@ -53,6 +53,7 @@ public class QueryException {
   public static final int BROKER_TIMEOUT_ERROR_CODE = 400;
   public static final int BROKER_RESOURCE_MISSING_ERROR_CODE = 410;
   public static final int BROKER_INSTANCE_MISSING_ERROR_CODE = 420;
+  public static final int BROKER_REQUEST_SEND_ERROR_CODE = 425;
   public static final int TOO_MANY_REQUESTS_ERROR_CODE = 429;
   public static final int INTERNAL_ERROR_CODE = 450;
   public static final int MERGE_RESPONSE_ERROR_CODE = 500;
@@ -125,8 +126,17 @@ public class QueryException {
   }
 
   public static ProcessingException getException(ProcessingException 
processingException, Exception exception) {
+    return getException(processingException, 
getTruncatedStackTrace(exception));
+  }
+
+  public static ProcessingException getException(ProcessingException 
processingException, String errorMessage) {
     String errorType = processingException.getMessage();
     ProcessingException copiedProcessingException = 
processingException.deepCopy();
+    copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
+    return copiedProcessingException;
+  }
+
+  public static String getTruncatedStackTrace(Exception exception) {
     StringWriter stringWriter = new StringWriter();
     exception.printStackTrace(new PrintWriter(stringWriter));
     String fullStackTrace = stringWriter.toString();
@@ -136,15 +146,7 @@ public class QueryException {
     for (int i = 0; i < numLinesOfStackTrace; i++) {
       lengthOfStackTrace += lines[i].length();
     }
-    copiedProcessingException.setMessage(errorType + ":\n" + 
fullStackTrace.substring(0, lengthOfStackTrace));
-    return copiedProcessingException;
-  }
-
-  public static ProcessingException getException(ProcessingException 
processingException, String errorMessage) {
-    String errorType = processingException.getMessage();
-    ProcessingException copiedProcessingException = 
processingException.deepCopy();
-    copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
-    return copiedProcessingException;
+    return fullStackTrace.substring(0, lengthOfStackTrace);
   }
 
   /**
diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/transport/AsyncQueryResponse.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/transport/AsyncQueryResponse.java
index baed7da..8ca2510 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/transport/AsyncQueryResponse.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/transport/AsyncQueryResponse.java
@@ -39,6 +39,8 @@ public class AsyncQueryResponse {
   private final CountDownLatch _countDownLatch;
   private final long _maxEndTimeMs;
 
+  private volatile Exception _brokerRequestSendException;
+
   public AsyncQueryResponse(QueryRouter queryRouter, long requestId, 
Set<ServerRoutingInstance> serversQueried,
       long startTimeMs, long timeoutMs) {
     _queryRouter = queryRouter;
@@ -105,4 +107,12 @@ public class AsyncQueryResponse {
       markQueryFailed();
     }
   }
+
+  public Exception getBrokerRequestSendException() {
+    return _brokerRequestSendException;
+  }
+
+  void setBrokerRequestSendException(Exception brokerRequestSendException) {
+    _brokerRequestSendException = brokerRequestSendException;
+  }
 }
diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/transport/QueryRouter.java 
b/pinot-core/src/main/java/org/apache/pinot/core/transport/QueryRouter.java
index 60f4a42..2710a16 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/transport/QueryRouter.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/transport/QueryRouter.java
@@ -92,6 +92,7 @@ public class QueryRouter {
         LOGGER.error("Caught exception while sending request {} to server: {}, 
marking query failed", requestId,
             serverRoutingInstance, e);
         _brokerMetrics.addMeteredTableValue(rawTableName, 
BrokerMeter.REQUEST_SEND_EXCEPTIONS, 1);
+        asyncQueryResponse.setBrokerRequestSendException(e);
         asyncQueryResponse.markQueryFailed();
         break;
       }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to