[server][da-vinci-client] Add compaction cancelled metric, add detail…

…s to node_removable (#734) Added compaction cancellation metric to get more insight on rocksdb SST file size usage. Also adds more details to node removable result to indicate why a node considered removable as passing wrong ID silently tells its removable. --------- Co-authored-by: Sourav Maji <[email protected]>
linkedin · Nov 3, 2023 · 0a04e37 · 0a04e37
1 parent 6709c26
commit 0a04e37
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 7 deletions.
diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/RocksDBMemoryStats.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/RocksDBMemoryStats.java
@@ -38,6 +38,7 @@ public class RocksDBMemoryStats extends AbstractVeniceStats {
       "rocksdb.cur-size-all-mem-tables",
       "rocksdb.size-all-mem-tables",
       "rocksdb.num-entries-active-mem-table",
+      "rocksdb.compaction.cancelled",
       "rocksdb.num-entries-imm-mem-tables",
       "rocksdb.num-deletes-active-mem-table",
       "rocksdb.num-deletes-imm-mem-tables",

diff --git a/...venice-controller/src/main/java/com/linkedin/venice/controller/InstanceStatusDecider.java b/...venice-controller/src/main/java/com/linkedin/venice/controller/InstanceStatusDecider.java
@@ -67,7 +67,8 @@ static NodeRemovableResult isRemovable(
     try {
       // If instance is not alive, it's removable.
       if (!HelixUtils.isLiveInstance(clusterName, instanceId, resources.getHelixManager())) {
-        return NodeRemovableResult.removableResult();
+        return NodeRemovableResult
+            .removableResult("Instance " + instanceId + " not found in liveinstance set of cluster " + clusterName);
       }
 
       RoutingDataRepository routingDataRepository = resources.getRoutingDataRepository();
@@ -111,7 +112,7 @@ static NodeRemovableResult isRemovable(
                   resourceName,
                   clusterName,
                   result.getSecond());
-              return NodeRemovableResult.nonremoveableResult(
+              return NodeRemovableResult.nonRemovableResult(
                   resourceName,
                   NodeRemovableResult.BlockingRemoveReason.WILL_LOSE_DATA,
                   result.getSecond());
@@ -137,15 +138,15 @@ static NodeRemovableResult isRemovable(
                   resourceName,
                   clusterName,
                   result.getSecond());
-              return NodeRemovableResult.nonremoveableResult(
+              return NodeRemovableResult.nonRemovableResult(
                   resourceName,
                   NodeRemovableResult.BlockingRemoveReason.WILL_TRIGGER_LOAD_REBALANCE,
                   result.getSecond());
             }
           }
         }
       }
-      return NodeRemovableResult.removableResult();
+      return NodeRemovableResult.removableResult("Instance " + instanceId + " can be removed.");
     } catch (Exception e) {
       String errorMsg = "Can not verify whether instance " + instanceId + " is removable.";
       LOGGER.error(errorMsg, e);

diff --git a/...s/venice-controller/src/main/java/com/linkedin/venice/controller/NodeRemovableResult.java b/...s/venice-controller/src/main/java/com/linkedin/venice/controller/NodeRemovableResult.java
@@ -26,14 +26,16 @@ public String getDetails() {
     return details;
   }
 
-  public static NodeRemovableResult removableResult() {
-    return new NodeRemovableResult();
+  public static NodeRemovableResult removableResult(String details) {
+    NodeRemovableResult result = new NodeRemovableResult();
+    result.details = details;
+    return result;
   }
 
   /**
    * @return a {@link NodeRemovableResult} object with specified parameters.
    */
-  public static NodeRemovableResult nonremoveableResult(
+  public static NodeRemovableResult nonRemovableResult(
       String blockingResource,
       BlockingRemoveReason blockingReason,
       String details) {