Removed forceRemove and guaranteesFalseNegatives

peteralfonsi · Oct 23, 2023 · 4abf6ea · 4abf6ea
1 parent 75be197
commit 4abf6ea
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 52 deletions.
diff --git a/server/src/main/java/org/opensearch/indices/KeyLookupStore.java b/server/src/main/java/org/opensearch/indices/KeyLookupStore.java
@@ -74,20 +74,6 @@ public interface KeyLookupStore<T> {
      */
     boolean remove(T value) throws Exception;
 
-
-    /**
-     * Remove the transformed version of this value from the store. Calling this function may cause
-     * contains() to return false negatives for future values.
-     * @param value The value to forcibly remove.
-     */
-    void forceRemove(T value) throws Exception;
-
-    /**
-     * Check if the object currently guarantees having no false negatives when running contains().
-     * @return false if there will not be false negatives, true if there could be false negatives.
-     */
-    boolean canHaveFalseNegatives();
-
     /**
      * Returns the number of distinct values stored in the internal data structure.
      * Does not count values which weren't successfully added due to collisions.

diff --git a/server/src/main/java/org/opensearch/indices/RBMIntKeyLookupStore.java b/server/src/main/java/org/opensearch/indices/RBMIntKeyLookupStore.java
@@ -57,13 +57,12 @@ protected class KeyStoreStats {
         protected boolean guaranteesNoFalseNegatives;
         protected int maxNumEntries;
         protected boolean atCapacity;
-        protected CounterMetric numRemovalAttempts; // used in removable classes
+        protected CounterMetric numRemovalAttempts;
         protected CounterMetric numSuccessfulRemovals;
         protected KeyStoreStats(long memSizeCapInBytes, int maxNumEntries) {
             this.size = 0;
             this.numAddAttempts = new CounterMetric();
             this.numCollisions = new CounterMetric();
-            this.guaranteesNoFalseNegatives = true;
             this.memSizeCapInBytes = memSizeCapInBytes;
             this.maxNumEntries = maxNumEntries;
             this.atCapacity = false;
@@ -181,28 +180,6 @@ public boolean remove(Integer value) throws Exception {
         }
     }
 
-
-    @Override
-    public void forceRemove(Integer value) throws Exception {
-        if (value == null) {
-            return;
-        }
-        writeLock.lock();
-        stats.guaranteesNoFalseNegatives = false;
-        try {
-            int transformedValue = transform(value);
-            rbm.remove(transformedValue);
-            stats.size--;
-        } finally {
-            writeLock.unlock();
-        }
-    }
-
-    @Override
-    public boolean canHaveFalseNegatives() {
-        return !stats.guaranteesNoFalseNegatives;
-    }
-
     @Override
     public int getSize() {
         readLock.lock();

diff --git a/server/src/test/java/org/opensearch/indices/RBMIntKeyLookupStoreTests.java b/server/src/test/java/org/opensearch/indices/RBMIntKeyLookupStoreTests.java
@@ -41,7 +41,6 @@
 import java.util.concurrent.ThreadPoolExecutor;
 
 public class RBMIntKeyLookupStoreTests extends OpenSearchTestCase {
-    // Tests mostly based on HybridIntKeyStoreTests.java
     public void testInit() {
         long memCap = 100 * RBMSizeEstimator.BYTES_IN_MB;
         RBMIntKeyLookupStore kls = new RBMIntKeyLookupStore((int) Math.pow(2, 29), memCap);
@@ -66,18 +65,12 @@ public void testTransformationLogic() throws Exception {
         }
     }
 
-    public void testContainsAndForceRemove() throws Exception {
+    public void testContains() throws Exception {
         RBMIntKeyLookupStore kls = new RBMIntKeyLookupStore((int) Math.pow(2, 29), 0L);
         for (int i = 0; i < 2000; i++) {
             kls.add(i);
             assertTrue(kls.contains(i));
         }
-        assertFalse(kls.canHaveFalseNegatives());
-        for (int i = 1900; i < 2000; i++) {
-            kls.forceRemove(i);
-            assertFalse(kls.contains(i));
-        }
-        assertEquals(1900, kls.getSize());
     }
 
     public void testAddingStatsGetters() throws Exception {
@@ -145,8 +138,6 @@ public void testMemoryCapBlocksAdd() throws Exception {
 
     public void testConcurrency() throws Exception {
         Random rand = Randomness.get();
-        int modulo = (int) Math.pow(2, 29);
-        long memCap = 100 * RBMSizeEstimator.BYTES_IN_MB;
         for (int j = 0; j < 5; j++) { // test with different numbers of threads
             RBMIntKeyLookupStore kls = new RBMIntKeyLookupStore((int) Math.pow(2, 29), 0L);
             int numThreads = rand.nextInt(50) + 1;
@@ -274,8 +265,6 @@ public void testNullInputs() throws Exception {
         assertFalse(kls.contains(null));
         assertEquals(0, (int) kls.getInternalRepresentation(null));
         assertFalse(kls.remove(null));
-        kls.forceRemove(null);
-        assertFalse(kls.canHaveFalseNegatives());
         assertFalse(kls.isCollision(null, null));
         assertEquals(0, kls.getTotalAdds());
         Integer[] newVals = new Integer[]{1, 17, -2, null, -4, null};
@@ -300,8 +289,6 @@ public void testMemoryCapValueInitialization() {
             assertEquals(expectedMultipliers[i], kls.sizeEstimator.bufferMultiplier, delta);
             assertEquals(expectedSlopes[i], kls.sizeEstimator.slope, delta);
             assertEquals(expectedIntercepts[i], kls.sizeEstimator.intercept, delta);
-            System.out.println("log modulo: " + logModulos[i]);
-            System.out.println("Estimated size at 10^6: " + kls.sizeEstimator.getSizeInBytes(1000000));
         }
 
     }