SAMZA-2563: Fix double counting of InMemoryKeyValueStore deletion (#1458)
Symptom: Calling InMemoryKeyValueStore.delete will double count the deletes in metrics.
Cause: InMemoryKeyValueStore.delete is implemented by calling InMemoryKeyValueStore.put with a null value. InMemoryKeyValueStore.delete explicitly increments the delete metric. InMemoryKeyValueStore.put also explicitly increments the delete metric when passed a null value.
Changes: Remove incrementing the delete metric in InMemoryKeyValueStore.delete
diff --git a/samza-kv-inmemory/src/main/java/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.java b/samza-kv-inmemory/src/main/java/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.java
index 1d5a36c..3b1256d 100644
--- a/samza-kv-inmemory/src/main/java/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.java
+++ b/samza-kv-inmemory/src/main/java/org/apache/samza/storage/kv/inmemory/InMemoryKeyValueStore.java
@@ -87,8 +87,6 @@
@Override
public void delete(byte[] key) {
- // TODO Bug: SAMZA-2563: This double counts deletes for metrics, because put also counts a delete
- metrics.deletes().inc();
put(key, null);
}
diff --git a/samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java b/samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
index 48dbde6..7cdb235 100644
--- a/samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
+++ b/samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
@@ -239,11 +239,7 @@
this.inMemoryKeyValueStore.delete(key(0));
assertNull(this.inMemoryKeyValueStore.get(key(0)));
- /*
- * There is a bug in which deletes are double counted in metrics. This deletesCounter should only be invoked once
- * when the bug is fixed.
- */
- verify(this.deletesCounter, times(2)).inc();
+ verify(this.deletesCounter, times(1)).inc();
}
@Test
@@ -251,11 +247,8 @@
this.inMemoryKeyValueStore.delete(key(0));
assertNull(this.inMemoryKeyValueStore.get(key(0)));
- /*
- * There is a bug in which deletes are double counted in metrics. This deletesCounter should only be invoked once
- * when the bug is fixed.
- */
- verify(this.deletesCounter, times(2)).inc();
+
+ verify(this.deletesCounter, times(1)).inc();
}
@Test
