lhotari commented on code in PR #23634:
URL: https://github.com/apache/pulsar/pull/23634#discussion_r1860206850


##########
pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/VipStatus.java:
##########
@@ -38,25 +44,71 @@ public class VipStatus {
     public static final String ATTRIBUTE_STATUS_FILE_PATH = "statusFilePath";
     public static final String ATTRIBUTE_IS_READY_PROBE = "isReadyProbe";
 
+    // log a full thread dump when a deadlock is detected in status check once 
every 10 minutes
+    // to prevent excessive logging
+    private static final long LOG_THREADDUMP_INTERVAL_WHEN_DEADLOCK_DETECTED = 
600000L;
+    private static volatile long lastCheckStatusTimestamp;
+
+    // Rate limit status checks to every 500ms to prevent DoS
+    private static final long CHECK_STATUS_INTERVAL = 500L;
+    private static volatile boolean lastCheckStatusResult;
+
     @Context
     protected ServletContext servletContext;
 
     @GET
     public String checkStatus() {
-        String statusFilePath = (String) 
servletContext.getAttribute(ATTRIBUTE_STATUS_FILE_PATH);
-        @SuppressWarnings("unchecked")
-        Supplier<Boolean> isReadyProbe = (Supplier<Boolean>) 
servletContext.getAttribute(ATTRIBUTE_IS_READY_PROBE);
+        // Locking classes to avoid deadlock detection in multi-thread 
concurrent requests.
+        synchronized (VipStatus.class) {
+            if (System.currentTimeMillis() - lastCheckStatusTimestamp < 
CHECK_STATUS_INTERVAL) {
+                lastCheckStatusTimestamp = System.currentTimeMillis();
+                if (lastCheckStatusResult) {
+                    return "OK";
+                } else {
+                    throw new 
WebApplicationException(Status.SERVICE_UNAVAILABLE);
+                }
+            }
+            lastCheckStatusTimestamp = System.currentTimeMillis();

Review Comment:
   This should be the only location where `lastCheckStatusTimestamp` is updated.



##########
pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/VipStatus.java:
##########
@@ -38,25 +44,71 @@ public class VipStatus {
     public static final String ATTRIBUTE_STATUS_FILE_PATH = "statusFilePath";
     public static final String ATTRIBUTE_IS_READY_PROBE = "isReadyProbe";
 
+    // log a full thread dump when a deadlock is detected in status check once 
every 10 minutes
+    // to prevent excessive logging
+    private static final long LOG_THREADDUMP_INTERVAL_WHEN_DEADLOCK_DETECTED = 
600000L;
+    private static volatile long lastCheckStatusTimestamp;
+
+    // Rate limit status checks to every 500ms to prevent DoS
+    private static final long CHECK_STATUS_INTERVAL = 500L;
+    private static volatile boolean lastCheckStatusResult;
+
     @Context
     protected ServletContext servletContext;
 
     @GET
     public String checkStatus() {
-        String statusFilePath = (String) 
servletContext.getAttribute(ATTRIBUTE_STATUS_FILE_PATH);
-        @SuppressWarnings("unchecked")
-        Supplier<Boolean> isReadyProbe = (Supplier<Boolean>) 
servletContext.getAttribute(ATTRIBUTE_IS_READY_PROBE);
+        // Locking classes to avoid deadlock detection in multi-thread 
concurrent requests.
+        synchronized (VipStatus.class) {
+            if (System.currentTimeMillis() - lastCheckStatusTimestamp < 
CHECK_STATUS_INTERVAL) {
+                lastCheckStatusTimestamp = System.currentTimeMillis();

Review Comment:
   I hope this clarifies: 
https://github.com/apache/pulsar/pull/23634/files#r1860206850



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

Reply via email to