[GitHub] [geode] albertogpz commented on a diff in pull request #7747: GEODE-10334: Send alert when thread stuck for long

[GitBox <gi...@apache.org>]

albertogpz commented on code in PR #7747:
URL: https://github.com/apache/geode/pull/7747#discussion_r895878888


##########
geode-core/src/main/java/org/apache/geode/internal/monitoring/executor/AbstractExecutor.java:
##########
@@ -51,9 +56,25 @@ protected AbstractExecutor(String groupName, long threadID) {
 
   public void handleExpiry(long stuckTime, Map<Long, ThreadInfo> threadInfoMap) {
     incNumIterationsStuck();
+    sendAlertForThreadStuckForLong(stuckTime, threadInfoMap);
     logger.warn(createThreadReport(stuckTime, threadInfoMap));
   }
 
+  private void sendAlertForThreadStuckForLong(long stuckTime, Map<Long, ThreadInfo> threadInfoMap) {
+    if (maxThreadStuckTime <= 0) {
+      return;
+    }
+    if (threadInfoMap.get(threadID) == null) {
+      return;
+    }
+    if (stuckForGood) {
+      return;
+    }
+    if (stuckTime > maxThreadStuckTime) {
+      stuckForGood = true;
+      logger.fatal(createThreadReport(stuckTime, threadInfoMap));

Review Comment:
   @kirklund I think the alert will not be generated by default if `error` is used. In order to see the alert when using `error`, the `alertLevel` in the `DistributedMXBean` should be changed from `severe` to a lower level.
   
   See https://geode.apache.org/docs/guide/114/managing/management/notification_federation_and_alerts.html
   
   Anyhow, I agree with you that it makes sense to use `error` given that having a thread stuck is not necessarily a catastrophic failure that may result in data corruption.



-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org