Github user joshelser commented on a diff in the pull request: https://github.com/apache/phoenix/pull/315#discussion_r205884335 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java --- @@ -204,6 +219,39 @@ private static void resetGlobalMetrics() { } } + private boolean verifyMetricsFromSink() throws InterruptedException { + Map<String, Long> expectedMetrics = new HashMap<>(); + for (GlobalMetric m : PhoenixRuntime.getGlobalPhoenixClientMetrics()) { + expectedMetrics.put(m.getMetricType().name(), m.getTotalSum()); + } + + for (int i = 0; i < MAX_RETRIES; i++) { + LOG.info("Verifying Global Metrics from Hadoop Sink, Retry: " + (i + 1)); + if (verifyMetricsFromSinkOnce(expectedMetrics)) { + LOG.info("Values from Hadoop Metrics Sink match actual values"); + return true; + } + Thread.sleep(1000); + } + return false; + } + + private boolean verifyMetricsFromSinkOnce(Map<String, Long> expectedMetrics) { + synchronized (GlobalPhoenixMetricsTestSink.lock) { + for (AbstractMetric metric : GlobalPhoenixMetricsTestSink.metrics) { + if (expectedMetrics.containsKey(metric.name())) { --- End diff -- This check doesn't catch the case where you have `expectedMetrics` but `GlobalPhoenixMetricsTestSink.metrics` is empty. I'd switch this around to iterate over your expectations, ensuring that they all exist.
---