xinglin commented on code in PR #6146:
URL: https://github.com/apache/hadoop/pull/6146#discussion_r1347672845


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:
##########
@@ -568,29 +579,15 @@ public long getPurgeIntervalNanos() {
    * @param methodName - RPC Request method name
    * @param details - Processing Detail.
    *
-   * if this request took too much time relative to other requests
-   * we consider that as a slow RPC. 3 is a magic number that comes
-   * from 3 sigma deviation. A very simple explanation can be found
-   * by searching for 68-95-99.7 rule. We flag an RPC as slow RPC
-   * if and only if it falls above 99.7% of requests. We start this logic
-   * only once we have enough sample size.
+   * if this request took time exceed `logSlowRPCThresholdMs`
+   * we consider that as a slow RPC.
    */
   void logSlowRpcCalls(String methodName, Call call,
       ProcessingDetails details) {
-    final int deviation = 3;
-
-    // 1024 for minSampleSize just a guess -- not a number computed based on
-    // sample size analysis. It is chosen with the hope that this
-    // number is high enough to avoid spurious logging, yet useful
-    // in practice.
-    final int minSampleSize = 1024;
-    final double threeSigma = rpcMetrics.getProcessingMean() +
-        (rpcMetrics.getProcessingStdDev() * deviation);
 
     long processingTime =
             details.get(Timing.PROCESSING, rpcMetrics.getMetricsTimeUnit());
-    if ((rpcMetrics.getProcessingSampleCount() > minSampleSize) &&
-        (processingTime > threeSigma)) {
+    if (processingTime > getLogSlowRPCThresholdMs()) {

Review Comment:
   We should keep both methods, rather than replacing the old one with the new 
one. We should do something like the following. 
   ```
   long processingTimeThreshold = LONG.MAX;
   if (slow_rpc_based_on_hard_threshold) {
      processingTimeThreshold = thresholdFromConfig 
   } else {
      processingTimeThreshold = threeSigma;
   }
   
   if (processingTime > processingTimeThreshold) {
     ....
   }
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to