From b039f7591f7a9988ca283da0e15aec1ac858e24c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A1rton=20Elek?= <elek@apache.org>
Date: Thu, 13 Jun 2019 16:27:23 +0200
Subject: [PATCH] HDDS-1682. TestEventWatcher.testMetrics is flaky

Closes #962.
---
 .../hdds/server/events/EventWatcher.java      | 11 +++++-----
 .../hdds/server/events/TestEventWatcher.java  | 22 ++++++++++++++-----
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/events/EventWatcher.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/events/EventWatcher.java
index a118c3ed607..301c71e8775 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/events/EventWatcher.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/events/EventWatcher.java
@@ -143,14 +143,15 @@ public abstract class EventWatcher<TIMEOUT_PAYLOAD extends
   protected synchronized void handleCompletion(COMPLETION_PAYLOAD
       completionPayload, EventPublisher publisher) throws
       LeaseNotFoundException {
-    metrics.incrementCompletedEvents();
     long id = completionPayload.getId();
     leaseManager.release(id);
     TIMEOUT_PAYLOAD payload = trackedEventsByID.remove(id);
-    trackedEvents.remove(payload);
-    long originalTime = startTrackingTimes.remove(id);
-    metrics.updateFinishingTime(System.currentTimeMillis() - originalTime);
-    onFinished(publisher, payload);
+    if (trackedEvents.remove(payload)) {
+      metrics.incrementCompletedEvents();
+      long originalTime = startTrackingTimes.remove(id);
+      metrics.updateFinishingTime(System.currentTimeMillis() - originalTime);
+      onFinished(publisher, payload);
+    }
   }
 
   private synchronized void handleTimeout(EventPublisher publisher,
diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/events/TestEventWatcher.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/events/TestEventWatcher.java
index 88ac3786c73..dcbcdb03f96 100644
--- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/events/TestEventWatcher.java
+++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/events/TestEventWatcher.java
@@ -179,22 +179,32 @@ public class TestEventWatcher {
 
     queue.fireEvent(REPLICATION_COMPLETED, event1Completed);
 
-    Thread.sleep(2200L);
+    //lease manager timeout = 2000L
+    Thread.sleep(3 * 2000L);
+
+    queue.processAll(2000L);
 
     //until now: 3 in-progress activities are tracked with three
     // UnderreplicatedEvents. The first one is completed, the remaining two
-    // are timed out (as the timeout -- defined in the leasmanager -- is 2000ms.
+    // are timed out (as the timeout -- defined in the lease manager -- is
+    // 2000ms).
 
     EventWatcherMetrics metrics = replicationWatcher.getMetrics();
 
     //3 events are received
     Assert.assertEquals(3, metrics.getTrackedEvents().value());
 
-    //one is finished. doesn't need to be resent
-    Assert.assertEquals(1, metrics.getCompletedEvents().value());
+    //completed + timed out = all messages
+    Assert.assertEquals(
+        "number of timed out and completed messages should be the same as the"
+            + " all messages",
+        metrics.getTrackedEvents().value(),
+        metrics.getCompletedEvents().value() + metrics.getTimedOutEvents()
+            .value());
 
-    //Other two are timed out and resent
-    Assert.assertEquals(2, metrics.getTimedOutEvents().value());
+    //_at least_ two are timed out.
+    Assert.assertTrue("At least two events should be timed out.",
+        metrics.getTimedOutEvents().value() >= 2);
 
     DefaultMetricsSystem.shutdown();
   }