You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by bh...@apache.org on 2021/02/04 04:39:10 UTC

[samza] branch master updated: SAMZA-2563: Fix double counting of InMemoryKeyValueStore deletion (#1458)

This is an automated email from the ASF dual-hosted git repository.

bharathkk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/samza.git


The following commit(s) were added to refs/heads/master by this push:
     new 4e7154f  SAMZA-2563: Fix double counting of InMemoryKeyValueStore deletion (#1458)
4e7154f is described below

commit 4e7154f11b9c2ad33464b08f46900644944a0682
Author: Ziting <90...@users.noreply.github.com>
AuthorDate: Wed Feb 3 20:39:02 2021 -0800

    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
---
 .../samza/storage/kv/inmemory/InMemoryKeyValueStore.java    |  2 --
 .../storage/kv/inmemory/TestInMemoryKeyValueStore.java      | 13 +++----------
 2 files changed, 3 insertions(+), 12 deletions(-)

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 @@ public class InMemoryKeyValueStore implements KeyValueStore<byte[], byte[]> {
 
   @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 @@ public class TestInMemoryKeyValueStore {
     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 @@ public class TestInMemoryKeyValueStore {
     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