From 7eb99f375320d46f81fdc105919fdb937fc2be92 Mon Sep 17 00:00:00 2001 From: ganzichen Date: Tue, 4 Jun 2024 15:44:26 +0800 Subject: [PATCH 1/3] feature: remove namespace when root path delete --- .../prometheus/PrometheusMetricsProvider.java | 10 ++ .../PrometheusMetricsProviderTest.java | 116 +++++++++++++++++- .../apache/zookeeper/metrics/CounterSet.java | 10 +- .../apache/zookeeper/metrics/SummarySet.java | 10 +- .../metrics/impl/NullMetricsProvider.java | 8 ++ .../org/apache/zookeeper/server/DataTree.java | 28 +++-- .../zookeeper/server/ZooKeeperServer.java | 7 ++ .../server/metric/AvgMinMaxCounterSet.java | 8 ++ .../metric/AvgMinMaxPercentileCounterSet.java | 8 ++ .../server/metric/SimpleCounterSet.java | 8 ++ .../zookeeper/server/ZooKeeperServerTest.java | 69 +++++++++++ .../metric/AvgMinMaxCounterSetTest.java | 32 +++++ .../server/metric/MetricSetRemoveTest.java | 70 +++++++++++ 13 files changed, 374 insertions(+), 10 deletions(-) create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java index af4bff9d218..5ab0430b875 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java +++ b/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); diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java index 59115e31d90..1d3fd9c7d85 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java +++ b/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 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 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 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 expectedNames = Collections.singletonList(String.format("# TYPE %s summary", name)); + final List 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 expectedNames) throws Exception { + // validate with dump call after remove + final Map expectedNoMetricsMap = new HashMap<>(); + validateWithDump(expectedNoMetricsMap); + + // validate with servlet call after remove + final List expectedNoMetrics = new ArrayList<>(); + validateWithServletCall(expectedNames, expectedNoMetrics); + } + @Test public void testSummarySet() throws Exception { final String name = "ss"; diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/CounterSet.java b/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/CounterSet.java index c9c7c13c489..ab796db0aad 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/CounterSet.java +++ b/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 *

This method is thread safe, The MetricsProvider will take care of synchronization.

* - * @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 + *

This method is thread safe, The MetricsProvider will take care of synchronization.

+ * + * @param key the key to remove for the given key + */ + void remove(String key); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/SummarySet.java b/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/SummarySet.java index 8386d185fa0..d92fd5b0c24 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/SummarySet.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/SummarySet.java @@ -29,9 +29,17 @@ public interface SummarySet { * Register a value. *

This method is thread safe, The MetricsProvider will take care of synchronization.

* - * @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. + *

This method is thread safe, The MetricsProvider will take care of synchronization.

+ * + * @param key the key to remove the Summary for the given key + */ + void remove(String key); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/NullMetricsProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/NullMetricsProvider.java index 8d07e91865d..6dddc254d51 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/NullMetricsProvider.java +++ b/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) { + } + } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java index 012a10e5469..ca3b46082f8 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataTree.java +++ b/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 { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java index 19f1fac4536..2ad438973cf 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java @@ -2416,5 +2416,12 @@ static void updateQuotaExceededMetrics(final String namespace) { } ServerMetrics.getMetrics().QUOTA_EXCEEDED_ERROR_PER_NAMESPACE.add(namespace, 1); } + + static void removeQuotaExceededMetrics(final String namespace) { + if (namespace == null) { + return; + } + ServerMetrics.getMetrics().QUOTA_EXCEEDED_ERROR_PER_NAMESPACE.remove(namespace); + } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSet.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSet.java index 8696950ebfa..271f5507d25 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSet.java +++ b/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 values() { Map m = new LinkedHashMap<>(); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxPercentileCounterSet.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxPercentileCounterSet.java index a6569956cc3..5b68336c950 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxPercentileCounterSet.java +++ b/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 values() { Map m = new LinkedHashMap<>(); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounterSet.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounterSet.java index 63166de3f20..54be563b635 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounterSet.java +++ b/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); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java index 6a6cfd7ab59..8d716a301fb 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java @@ -221,6 +221,75 @@ public void testUpdateQuotaExceededMetrics() { assertEquals(count, values.get(String.format("%s_%s", namespace, name))); } + @Test + public void updateQuotaExceededMetricsRemoveTest() { + final String name = QuotaMetricsUtils.QUOTA_EXCEEDED_ERROR_PER_NAMESPACE; + final String[] namespaces = new String[3]; + final long count = 3L; + + // updateQuotaExceededMetrics multi times namespaces + for (int i = 0; i < count; i++) { + String namespace = UUID.randomUUID().toString(); + ZooKeeperServer.updateQuotaExceededMetrics(namespace); + namespaces[i] = namespace; + } + + // validate updateQuotaExceededMetrics success + final Map values = MetricsUtils.currentServerMetrics(); + for (int i = 0; i < count; i++) { + String namespace = namespaces[i]; + assertEquals(1, values.keySet().stream().filter( + key -> key.contains(String.format("%s_%s", namespace, name))).count()); + assertEquals(1L, values.get(String.format("%s_%s", namespace, name))); + } + + // removeQuotaExceededMetrics multi times exists namespaces + for (int i = 0; i < count; i++) { + String namespace = namespaces[i]; + ZooKeeperServer.removeQuotaExceededMetrics(namespace); + } + + // validate removeQuotaExceededMetrics success + final Map noValues = MetricsUtils.currentServerMetrics(); + for (int i = 0; i < count; i++) { + String namespace = namespaces[i]; + assertEquals(0, noValues.keySet().stream().filter( + key -> key.contains(String.format("%s_%s", namespace, name))).count()); + assertEquals(null, noValues.get(String.format("%s_%s", namespace, name))); + } + + // removeQuotaExceededMetrics multi times no exists namespaces + for (int i = 0; i < count; i++) { + String namespace = namespaces[i]; + ZooKeeperServer.removeQuotaExceededMetrics(namespace); + } + ZooKeeperServer.removeQuotaExceededMetrics(null); + } + + @Test + public void updateQuotaExceededMetricsRemoveNoExistsTest() { + final String[] namespaces = new String[3]; + final long count = 3L; + + // create multi times namespaces + for (int i = 0; i < count; i++) { + String namespace = UUID.randomUUID().toString(); + namespaces[i] = namespace; + } + + // removeQuotaExceededMetrics multi times no exists namespaces + for (int i = 0; i < count; i++) { + String namespace = namespaces[i]; + ZooKeeperServer.removeQuotaExceededMetrics(namespace); + } + } + + @Test + public void updateQuotaExceededMetricsRemoveNullKeyTest() { + // removeQuotaExceededMetrics with null key and make sure no exception is thrown + ZooKeeperServer.removeQuotaExceededMetrics(null); + } + @Test public void testUpdateQuotaExceededMetrics_nullNamespace() { assertDoesNotThrow(() -> ZooKeeperServer.updateQuotaExceededMetrics(null)); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSetTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSetTest.java index 7201ca9c20d..9f76e432b45 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/AvgMinMaxCounterSetTest.java +++ b/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 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 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(); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java new file mode 100644 index 00000000000..dd7e40964cb --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.metric; + +import static org.apache.zookeeper.client.FourLetterWordMain.send4LetterWord; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.ZooDefs.Ids; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.test.ClientBase; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class MetricSetRemoveTest extends ClientBase { + + protected static final Logger LOG = LoggerFactory.getLogger(MetricSetRemoveTest.class); + + @Test + public void nameSpaceMetricRemoveTest() throws Exception { + CountdownWatcher watcher = new CountdownWatcher(); + ZooKeeper zk = new ZooKeeper(hostPort, 10000, watcher); + // Create and immediately delete 100 root nodes + String testPath = "summarySetTest"; + for (int i = 0; i < 100; i++) { + String path = "/" + testPath + i; + zk.create(path, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.delete(path, -1); + } + + String host = hostPort.split(":")[0]; + int port = Integer.valueOf(hostPort.split(":")[1]); + String mntrCmd = send4LetterWord(host, port, "mntr"); + String metricTail = "_write_per_namespace"; + Set mntrLines = Arrays.asList(mntrCmd.split("\n")).stream() + .filter(line -> line.contains(metricTail)) + .map(line -> line.split("\t")[0]) + .collect(Collectors.toSet()); + + // Verify that the metrics for the created and deleted nodes do not appear in the 'mntr' namespace output + for (int i = 0; i < 100; i++) { + for (String metricHead : Arrays.asList("zk_cnt_", "zk_sum_", "zk_avg_", "zk_min_", "zk_max_")) { + String metric = metricHead + testPath + i + metricTail; + assertFalse(mntrLines.contains(metric)); + } + } + zk.close(); + } +} From 1ee33815ef801af218a15d09929cdb9fc79b1515 Mon Sep 17 00:00:00 2001 From: ganzichen Date: Tue, 4 Jun 2024 15:49:47 +0800 Subject: [PATCH 2/3] feature: remove unused method --- .../zookeeper/server/ZooKeeperServer.java | 7 -- .../zookeeper/server/ZooKeeperServerTest.java | 69 ------------------- 2 files changed, 76 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java index 2ad438973cf..19f1fac4536 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java @@ -2416,12 +2416,5 @@ static void updateQuotaExceededMetrics(final String namespace) { } ServerMetrics.getMetrics().QUOTA_EXCEEDED_ERROR_PER_NAMESPACE.add(namespace, 1); } - - static void removeQuotaExceededMetrics(final String namespace) { - if (namespace == null) { - return; - } - ServerMetrics.getMetrics().QUOTA_EXCEEDED_ERROR_PER_NAMESPACE.remove(namespace); - } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java index 8d716a301fb..6a6cfd7ab59 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ZooKeeperServerTest.java @@ -221,75 +221,6 @@ public void testUpdateQuotaExceededMetrics() { assertEquals(count, values.get(String.format("%s_%s", namespace, name))); } - @Test - public void updateQuotaExceededMetricsRemoveTest() { - final String name = QuotaMetricsUtils.QUOTA_EXCEEDED_ERROR_PER_NAMESPACE; - final String[] namespaces = new String[3]; - final long count = 3L; - - // updateQuotaExceededMetrics multi times namespaces - for (int i = 0; i < count; i++) { - String namespace = UUID.randomUUID().toString(); - ZooKeeperServer.updateQuotaExceededMetrics(namespace); - namespaces[i] = namespace; - } - - // validate updateQuotaExceededMetrics success - final Map values = MetricsUtils.currentServerMetrics(); - for (int i = 0; i < count; i++) { - String namespace = namespaces[i]; - assertEquals(1, values.keySet().stream().filter( - key -> key.contains(String.format("%s_%s", namespace, name))).count()); - assertEquals(1L, values.get(String.format("%s_%s", namespace, name))); - } - - // removeQuotaExceededMetrics multi times exists namespaces - for (int i = 0; i < count; i++) { - String namespace = namespaces[i]; - ZooKeeperServer.removeQuotaExceededMetrics(namespace); - } - - // validate removeQuotaExceededMetrics success - final Map noValues = MetricsUtils.currentServerMetrics(); - for (int i = 0; i < count; i++) { - String namespace = namespaces[i]; - assertEquals(0, noValues.keySet().stream().filter( - key -> key.contains(String.format("%s_%s", namespace, name))).count()); - assertEquals(null, noValues.get(String.format("%s_%s", namespace, name))); - } - - // removeQuotaExceededMetrics multi times no exists namespaces - for (int i = 0; i < count; i++) { - String namespace = namespaces[i]; - ZooKeeperServer.removeQuotaExceededMetrics(namespace); - } - ZooKeeperServer.removeQuotaExceededMetrics(null); - } - - @Test - public void updateQuotaExceededMetricsRemoveNoExistsTest() { - final String[] namespaces = new String[3]; - final long count = 3L; - - // create multi times namespaces - for (int i = 0; i < count; i++) { - String namespace = UUID.randomUUID().toString(); - namespaces[i] = namespace; - } - - // removeQuotaExceededMetrics multi times no exists namespaces - for (int i = 0; i < count; i++) { - String namespace = namespaces[i]; - ZooKeeperServer.removeQuotaExceededMetrics(namespace); - } - } - - @Test - public void updateQuotaExceededMetricsRemoveNullKeyTest() { - // removeQuotaExceededMetrics with null key and make sure no exception is thrown - ZooKeeperServer.removeQuotaExceededMetrics(null); - } - @Test public void testUpdateQuotaExceededMetrics_nullNamespace() { assertDoesNotThrow(() -> ZooKeeperServer.updateQuotaExceededMetrics(null)); From 7a98d7edc2c726cfc81c6b5887964b09aa177a43 Mon Sep 17 00:00:00 2001 From: ganzichen Date: Thu, 6 Jun 2024 14:42:14 +0800 Subject: [PATCH 3/3] fix: fixing unit tests --- .../test/java/org/apache/zookeeper/server/DataTreeTest.java | 5 ++--- .../apache/zookeeper/server/metric/MetricSetRemoveTest.java | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java index 07a69f14f01..e85c941643b 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/DataTreeTest.java @@ -572,13 +572,11 @@ 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 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")); @@ -586,6 +584,7 @@ public void testDataTreeMetrics() throws Exception { 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); } /** diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java index dd7e40964cb..d41c159af4a 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/metric/MetricSetRemoveTest.java @@ -20,11 +20,9 @@ import static org.apache.zookeeper.client.FourLetterWordMain.send4LetterWord; import static org.junit.jupiter.api.Assertions.assertFalse; - import java.util.Arrays; import java.util.Set; import java.util.stream.Collectors; - import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.ZooDefs.Ids; import org.apache.zookeeper.ZooKeeper;