virajjasani commented on a change in pull request #754: HBASE-22978 : Online
slow response log
URL: https://github.com/apache/hbase/pull/754#discussion_r382539331
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##########
@@ -496,11 +517,63 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+ final String tag = (tooLarge && tooSlow) ? "TooLarge & TooSlow"
+ : (tooSlow ? "TooSlow" : "TooLarge");
LOG.warn("(response" + tag + "): " + GSON.toJson(responseInfo));
+ if (tooSlow && this.slowLogRecorder != null) {
+ logOnlineSlowResponse(param, methodName, call,
+ clientAddress, startTime, processingTime, qTime, responseSize,
userName,
+ className, responseInfo);
+ }
+ }
+
+ /**
+ * Add too slow log to ringbuffer for retrieval of latest n slow logs
+ *
+ * @param param The parameters received in the call
+ * @param methodName The name of the method invoked
+ * @param call The string representation of the call
+ * @param clientAddress The address of the client who made this call
+ * @param startTime The time that the call was initiated, in ms
+ * @param processingTime The duration that the call took to run, in ms
+ * @param qTime The duration that the call spent on the queue
+ * prior to being initiated, in ms
+ * @param responseSize The size in bytes of the response buffer
+ * @param userName UserName of the current RPC Call
+ * @param className ClassName of the SlowLog call
+ * @param responseInfo Base information map that is reported regardless of
type of call
+ */
+ private void logOnlineSlowResponse(Message param, String methodName, String
call,
+ String clientAddress, long startTime, int processingTime, int qTime,
long responseSize,
+ String userName, String className, Map<String, Object> responseInfo) {
+
+ try {
+
+ final SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails(call)
+ .setClientAddress(clientAddress)
+ .setMethodName(methodName)
+ .setMultiGets(responseInfo.containsKey(MULTI_GETS)
+ ? (int) responseInfo.get(MULTI_GETS) : 0)
+ .setMultiMutations(responseInfo.containsKey(MULTI_MUTATIONS)
+ ? (int) responseInfo.get(MULTI_MUTATIONS) : 0)
+ .setMultiServiceCalls(responseInfo.containsKey(MULTI_SERVICE_CALLS)
+ ? (int) responseInfo.get(MULTI_SERVICE_CALLS) : 0)
+ .setProcessingTime(processingTime)
+ .setQueueTime(qTime)
+ .setResponseSize(responseSize)
+ .setServerClass(className)
+ .setStartTime(startTime)
+ .setUserName(userName)
+ .build();
+ this.slowLogRecorder.addSlowLogPayload(slowLogPayload, param);
Review comment:
@bharathv The major points I have with all these comments on passing on
RpcCall object over to Ring Buffer:
1. Unnecessary holding object reference, but again we can remove reference
at consumer side of ring buffer as we discussed. However, RpcCall contains lot
of redundant info for ring buffer whereas the current implementation only has
required fields. We are not extracting info with this patch, we are just using
them.
2. Unit tests capability. Ring buffer producer API should be simple enough
for any one to just provide POJO with required fields rather than an RpcCall
(with lots of redundant stuffs) which we can't even Mock properly. With the
current implementation, if you take a look at `TestSlowLogRecorder`, we are
testing quite useful scenarios with that include `Disruptor + EvictingQueue`
usecases, which would not be easy to test with your suggestion of passing
RpcCall reference to producer of ring buffer.
To make it more readable, let me update private method
`logOnlineSlowResponse` to `logSlowResponseInRingBuffer` and update Javadoc
with enough info.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
