feature: remove namespace metric when root path delete

zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java

@@ -419,6 +419,11 @@ public void add(final String key, final long delta) {
                 LOG.error("invalid delta {} for metric {} with key {}", delta, name, key, e);
             }
         }
+
+        @Override
+        public void remove(String key) {
+            inner.remove(key);
+        }
     }
 
     private class PrometheusGaugeWrapper {
@@ -548,6 +553,11 @@ public void add(String key, long value) {
             reportMetrics(() -> observe(key, value));
         }
 
+        @Override
+        public void remove(String key) {
+            inner.remove(key);
+        }
+
         private void observe(final String key, final long value) {
             try {
                 inner.labels(key).observe(value);

zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java

@@ -220,6 +220,39 @@ public void testCounterSet_nullKey() {
         counterSet.add(null, 2);
     }
 
+    @Test
+    public void testCounterSetRemove() throws Exception {
+        // create and register a CounterSet
+        final String name = "testCounterSetRemove";
+        final CounterSet counterSet = provider.getRootContext().getCounterSet(name);
+        final String counterSetKey = "testNameSpace";
+        final List<String> expectedNames = Collections.singletonList(String.format("# TYPE %s counter", name));
+
+        counterSet.inc(counterSetKey);
+        counterSet.add(counterSetKey, 2);
+
+        // increment count then validate with dump call
+        final Map<String, Number> expectedMetricsMap = new HashMap<>();
+        for (final String key : Arrays.asList(counterSetKey)) {
+            expectedMetricsMap.put(String.format("%s{key=\"%s\"}", name, key), 3.0);
+        }
+        validateWithDump(expectedMetricsMap);
+
+        // remove count then validate with dump call
+        counterSet.remove(counterSetKey);
+        validateWithEmptyValues(expectedNames);
+    }
+
+    @Test
+    public void testCounterSetRemoveNullKey() {
+        // create and register a CounterSet
+        final String name = "testCounterSetRemove";
+        final CounterSet counterSet = provider.getRootContext().getCounterSet(name);
+
+        // remove count with null key and make sure no exception is thrown
+        counterSet.remove(null);
+    }
+
     @Test
     public void testGauge() throws Exception {
         int[] values = {78, -89};
@@ -437,11 +470,92 @@ public void testSummary_asyncAndExceedMaxQueueSize() throws Exception {
             }
         } finally {
             if (metricsProvider != null) {
-               metricsProvider.stop();
+                metricsProvider.stop();
             }
         }
     }
 
+    @Test
+    public void testSummarySetWithRemove() throws Exception {
+        final String name = "summarysetremove";
+        final String[] keys = {"ns1", "ns2"};
+        final double count = 3.0;
+
+        // create and register a SummarySet
+        final SummarySet summarySet = provider.getRootContext()
+                .getSummarySet(name, MetricsContext.DetailLevel.BASIC);
+
+        // update the SummarySet multiple times
+        for (int i = 0; i < count; i++) {
+            Arrays.asList(keys).forEach(key -> summarySet.add(key, 1));
+        }
+
+        // validate with dump call
+        final Map<String, Number> expectedMetricsMap = new HashMap<>();
+        for (final String key : keys) {
+            expectedMetricsMap.put(String.format("%s{key=\"%s\",quantile=\"0.5\"}", name, key), 1.0);
+            expectedMetricsMap.put(String.format("%s_count{key=\"%s\"}", name, key), count);
+            expectedMetricsMap.put(String.format("%s_sum{key=\"%s\"}", name, key), count);
+        }
+        validateWithDump(expectedMetricsMap);
+
+        // validate with servlet call
+        final List<String> expectedNames = Collections.singletonList(String.format("# TYPE %s summary", name));
+        final List<String> expectedMetrics = new ArrayList<>();
+        for (final String key : keys) {
+            expectedMetrics.add(String.format("%s{key=\"%s\",quantile=\"0.5\",} %s", name, key, 1.0));
+            expectedMetrics.add(String.format("%s_count{key=\"%s\",} %s", name, key, count));
+            expectedMetrics.add(String.format("%s_sum{key=\"%s\",} %s", name, key, count));
+        }
+        validateWithServletCall(expectedNames, expectedMetrics);
+
+        // remove the SummarySet
+        for (int i = 0; i < count; i++) {
+            Arrays.asList(keys).forEach(key -> summarySet.remove(key));
+        }
+        validateWithEmptyValues(expectedNames);
+    }
+
+    @Test
+    public void testSummarySetWithRemoveNoExistsKey() {
+        final String name = "summarysetremove";
+        final String[] keys = {"ns1", "ns2"};
+        final double count = 3.0;
+
+        // create and register a SummarySet
+        final SummarySet summarySet = provider.getRootContext()
+                .getSummarySet(name, MetricsContext.DetailLevel.BASIC);
+
+        // remove no exists key
+        for (int i = 0; i < count; i++) {
+            Arrays.asList(keys).forEach(key -> summarySet.remove(key));
+        }
+    }
+
+    @Test
+    public void testSummarySetWithRemoveNullKey() {
+        final String name = "summarysetremove";
+        final String[] keys = {"ns1", "ns2"};
+        final double count = 3.0;
+
+        // create and register a SummarySet
+        final SummarySet summarySet = provider.getRootContext()
+                .getSummarySet(name, MetricsContext.DetailLevel.BASIC);
+
+        // remove null
+        summarySet.remove(null);
+    }
+
+    private void validateWithEmptyValues(List<String> expectedNames) throws Exception {
+        // validate with dump call after remove
+        final Map<String, Number> expectedNoMetricsMap = new HashMap<>();
+        validateWithDump(expectedNoMetricsMap);
+
+        // validate with servlet call after remove
+        final List<String> expectedNoMetrics = new ArrayList<>();
+        validateWithServletCall(expectedNames, expectedNoMetrics);
+    }
+
     @Test
     public void testSummarySet() throws Exception {
         final String name = "ss";

zookeeper-server/src/main/java/org/apache/zookeeper/metrics/CounterSet.java

@@ -40,8 +40,16 @@ default void inc(String key) {
      * Increment the value by a given amount for the given key
      * <p>This method is thread safe, The MetricsProvider will take care of synchronization.</p>
      *
-     * @param key the key to increment the count for the given key
+     * @param key   the key to increment the count for the given key
      * @param delta amount to increment, this cannot be a negative number.
      */
     void add(String key, long delta);
+
+    /**
+     * Remove the vale for the given key
+     * <p>This method is thread safe, The MetricsProvider will take care of synchronization.</p>
+     *
+     * @param key the key to remove for the given key
+     */
+    void remove(String key);
 }

zookeeper-server/src/main/java/org/apache/zookeeper/metrics/SummarySet.java

@@ -29,9 +29,17 @@ public interface SummarySet {
      * Register a value.
      * <p>This method is thread safe, The MetricsProvider will take care of synchronization.</p>
      *
-     * @param key the key to access the Summary for the given key
+     * @param key   the key to access the Summary for the given key
      * @param value current value
      */
     void add(String key, long value);
 
+
+    /**
+     * Unregister a value.
+     * <p>This method is thread safe, The MetricsProvider will take care of synchronization.</p>
+     *
+     * @param key the key to remove the Summary for the given key
+     */
+    void remove(String key);
 }

zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/NullMetricsProvider.java

@@ -134,6 +134,10 @@ private static final class NullCounterSet implements CounterSet {
         @Override
         public void add(final String key, final long delta) {
         }
+
+        @Override
+        public void remove(String key) {
+        }
     }
 
     private static final class NullSummary implements Summary {
@@ -154,6 +158,10 @@ private static final class NullSummarySet implements SummarySet {
         public void add(String key, long value) {
         }
 
+        @Override
+        public void remove(String key) {
+        }
+
     }
 
 }

zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java

@@ -611,22 +611,36 @@ public void deleteNode(String path, long zxid) throws NoNodeException {
         }
 
         updateWriteStat(path, 0L);
+        // root node path is ""
+        if ("".equals(parentName)) {
+            removeNameSpace(path);
+        }
 
         if (LOG.isTraceEnabled()) {
             ZooTrace.logTraceMessage(
-                LOG,
-                ZooTrace.EVENT_DELIVERY_TRACE_MASK,
-                "dataWatches.triggerWatch " + path);
+                    LOG,
+                    ZooTrace.EVENT_DELIVERY_TRACE_MASK,
+                    "dataWatches.triggerWatch " + path);
             ZooTrace.logTraceMessage(
-                LOG,
-                ZooTrace.EVENT_DELIVERY_TRACE_MASK,
-                "childWatches.triggerWatch " + parentName);
+                    LOG,
+                    ZooTrace.EVENT_DELIVERY_TRACE_MASK,
+                    "childWatches.triggerWatch " + parentName);
         }
 
         WatcherOrBitSet processed = dataWatches.triggerWatch(path, EventType.NodeDeleted, zxid, acl);
         childWatches.triggerWatch(path, EventType.NodeDeleted, zxid, acl, processed);
         childWatches.triggerWatch("".equals(parentName) ? "/" : parentName,
-            EventType.NodeChildrenChanged, zxid, parentAcl);
+                EventType.NodeChildrenChanged, zxid, parentAcl);
+    }
+
+    private void removeNameSpace(String path) {
+        final String namespace = PathUtils.getTopNamespace(path);
+        if (namespace == null) {
+            return;
+        }
+        ServerMetrics.getMetrics().WRITE_PER_NAMESPACE.remove(namespace);
+        ServerMetrics.getMetrics().READ_PER_NAMESPACE.remove(namespace);
+        ServerMetrics.getMetrics().QUOTA_EXCEEDED_ERROR_PER_NAMESPACE.remove(namespace);
     }
 
     public Stat setData(String path, byte[] data, int version, long zxid, long time) throws NoNodeException {

zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSet.java

@@ -63,6 +63,14 @@ public void add(String key, long value) {
         addDataPoint(key, value);
     }
 
+    @Override
+    public void remove(String key) {
+        if (key == null) {
+            return;
+        }
+        counters.remove(key);
+    }
+
     @Override
     public Map<String, Object> values() {
         Map<String, Object> m = new LinkedHashMap<>();

zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxPercentileCounterSet.java

@@ -69,6 +69,14 @@ public void add(String key, long value) {
         addDataPoint(key, value);
     }
 
+    @Override
+    public void remove(String key) {
+        if (key == null) {
+            return;
+        }
+        counters.remove(key);
+    }
+
     @Override
     public Map<String, Object> values() {
         Map<String, Object> m = new LinkedHashMap<>();

zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounterSet.java

@@ -41,6 +41,14 @@ public void add(final String key, final long delta) {
         counter.add(delta);
     }
 
+    @Override
+    public void remove(String key) {
+        if (key == null) {
+            return;
+        }
+        counters.remove(key);
+    }
+
     @Override
     public void reset() {
         counters.values().forEach(SimpleCounter::reset);

zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java

@@ -572,20 +572,19 @@ public void testDataTreeMetrics() throws Exception {
         readBytes1 += CHILD2PATH.length() + DataTree.STAT_OVERHEAD_BYTES;
         dt.getChildren(TOP1PATH, new Stat(), null);
         readBytes1 += TOP1PATH.length() + CHILD1.length() + CHILD2.length() + DataTree.STAT_OVERHEAD_BYTES;
-        dt.deleteNode(TOP1PATH, 1);
-        writeBytes1 += TOP1PATH.length();
 
         Map<String, Object> values = MetricsUtils.currentServerMetrics();
         System.out.println("values:" + values);
         assertEquals(writeBytes1, values.get("sum_" + TOP1 + "_write_per_namespace"));
-        assertEquals(5L, values.get("cnt_" + TOP1 + "_write_per_namespace"));
+        assertEquals(4L, values.get("cnt_" + TOP1 + "_write_per_namespace"));
         assertEquals(writeBytes2, values.get("sum_" + TOP2 + "_write_per_namespace"));
         assertEquals(1L, values.get("cnt_" + TOP2 + "_write_per_namespace"));
 
         assertEquals(readBytes1, values.get("sum_" + TOP1 + "_read_per_namespace"));
         assertEquals(3L, values.get("cnt_" + TOP1 + "_read_per_namespace"));
         assertEquals(readBytes2, values.get("sum_" + TOP2 + "_read_per_namespace"));
         assertEquals(1L, values.get("cnt_" + TOP2 + "_read_per_namespace"));
+        dt.deleteNode(TOP1PATH, 1);
     }
 
     /**

zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSetTest.java

@@ -42,6 +42,38 @@ private void addDataPoints() {
         testCounterSet.add("key2", 5);
     }
 
+    @Test
+    public void testRemove() {
+        addDataPoints();
+
+        Map<String, Object> values = testCounterSet.values();
+
+        assertEquals(10, values.size(), "There should be 10 values in the set");
+        assertEquals(0.5D, values.get("avg_key1_test"), "avg_key1_test should =0.5");
+        assertEquals(0L, values.get("min_key1_test"), "min_key1_test should =0");
+        assertEquals(1L, values.get("max_key1_test"), "max_key1_test should =1");
+        assertEquals(2L, values.get("cnt_key1_test"), "cnt_key1_test should =2");
+        assertEquals(1L, values.get("sum_key1_test"), "sum_key1_test should =1");
+
+        assertEquals(3.5, values.get("avg_key2_test"), "avg_key2_test should =3.5");
+        assertEquals(2L, values.get("min_key2_test"), "min_key2_test should =2");
+        assertEquals(5L, values.get("max_key2_test"), "max_key2_test should =5");
+        assertEquals(4L, values.get("cnt_key2_test"), "cnt_key2_test should =4");
+        assertEquals(14L, values.get("sum_key2_test"), "sum_key2_test should =14");
+
+        testCounterSet.remove("key1");
+        testCounterSet.remove("key2");
+        Map<String, Object> valuesAfterRemove = testCounterSet.values();
+        assertEquals(0, valuesAfterRemove.size(), "testCounterSet should be 0 values in the set");
+
+        // test remove no exists key
+        testCounterSet.remove("key1");
+        testCounterSet.remove("key2");
+        // test remove null
+        testCounterSet.remove(null);
+        assertEquals(0, valuesAfterRemove.size(), "testCounterSet should be 0 values in the set");
+    }
+
     @Test
     public void testReset() {
         addDataPoints();