You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ri...@apache.org on 2021/07/01 20:22:32 UTC

[geode] branch GEODE-9375-Implement-ZRANGE-Radish-command updated (ad61dd7 -> 702a0e1)

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

ringles pushed a change to branch GEODE-9375-Implement-ZRANGE-Radish-command
in repository https://gitbox.apache.org/repos/asf/geode.git.


    from ad61dd7  Warning cleanup, remove unused constant/code
     new 8a0f35f  Clean up unused code
     new 515a47e  Clean up unused code
     new 702a0e1  Fix bug in NullRedisSet zadd, fix updated entries leaving fossils in tree

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../sortedset/AbstractZRangeIntegrationTest.java   |  9 +++++-
 .../internal/collections/IndexibleTreeSet.java     |  4 ---
 .../internal/collections/OrderStatisticsTree.java  | 33 ----------------------
 .../redis/internal/data/NullRedisSortedSet.java    |  2 +-
 .../geode/redis/internal/data/RedisSortedSet.java  |  5 +++-
 .../redis/internal/data/RedisSortedSetTest.java    | 11 +++++++-
 6 files changed, 23 insertions(+), 41 deletions(-)

[geode] 03/03: Fix bug in NullRedisSet zadd, fix updated entries leaving fossils in tree

Posted by ri...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ringles pushed a commit to branch GEODE-9375-Implement-ZRANGE-Radish-command
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 702a0e16feb22224c87d52db47313fadf5a77c66
Author: Ray Ingles <ri...@vmware.com>
AuthorDate: Thu Jul 1 16:19:11 2021 -0400

    Fix bug in NullRedisSet zadd, fix updated entries leaving fossils in tree
---
 .../executor/sortedset/AbstractZRangeIntegrationTest.java     |  9 ++++++++-
 .../apache/geode/redis/internal/data/NullRedisSortedSet.java  |  2 +-
 .../org/apache/geode/redis/internal/data/RedisSortedSet.java  |  5 ++++-
 .../apache/geode/redis/internal/data/RedisSortedSetTest.java  | 11 ++++++++++-
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
index d808d16..bcdfa60 100755
--- a/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
+++ b/geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/sortedset/AbstractZRangeIntegrationTest.java
@@ -82,7 +82,7 @@ public abstract class AbstractZRangeIntegrationTest implements RedisIntegrationT
   }
 
   @Test
-  public void shouldReturnSyntaxError_givenWrongWithScoresFlag() {
+  public void shouldReturnSyntaxError_givenInvalidWithScoresFlag() {
     jedis.zadd(SORTED_SET_KEY, 1.0, "member");
     assertThatThrownBy(
         () -> jedis.sendCommand(SORTED_SET_KEY, Protocol.Command.ZRANGE, SORTED_SET_KEY, "1", "2",
@@ -91,6 +91,13 @@ public abstract class AbstractZRangeIntegrationTest implements RedisIntegrationT
   }
 
   @Test
+  public void shouldNotRetainOldEntries_whenEntryUpdated() {
+    jedis.zadd(SORTED_SET_KEY, 2.0, "mem4");
+    assertThat(jedis.zcard(SORTED_SET_KEY)).isEqualTo(members.size());
+    assertThat(jedis.zrange(SORTED_SET_KEY, 0, 100).size()).isEqualTo(members.size());
+  }
+
+  @Test
   public void shouldReturnEmptyList_GivenInvalidRanges() {
     assertThat(jedis.zrange(SORTED_SET_KEY, 5, 0)).isEmpty();
     assertThat(jedis.zrange(SORTED_SET_KEY, 27, 30)).isEmpty();
diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
index 8028076..d74c89b 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
@@ -42,7 +42,7 @@ class NullRedisSortedSet extends RedisSortedSet {
       if (options.isINCR()) {
         return null;
       }
-      return 0L;
+      return 0;
     }
 
     if (options.isINCR()) {
diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
index 4f236af..a624248 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
@@ -171,11 +171,13 @@ public class RedisSortedSet extends AbstractRedisData {
     byte[] oldScore = null;
 
     OrderedSetEntry newEntry = new OrderedSetEntry(memberToAdd, scoreToAdd);
-    scoreSet.add(newEntry);
     OrderedSetEntry orderedSetEntry = members.put(memberToAdd, newEntry);
     if (orderedSetEntry == null) {
+      scoreSet.add(newEntry);
       sizeInBytes += calculateSizeOfFieldValuePair(memberToAdd, scoreToAdd);
     } else {
+      scoreSet.remove(orderedSetEntry);
+      scoreSet.add(newEntry);
       oldScore = orderedSetEntry.getScoreBytes();
       sizeInBytes += scoreToAdd.length - oldScore.length;
     }
@@ -350,6 +352,7 @@ public class RedisSortedSet extends AbstractRedisData {
     byte[] oldValue = null;
     OrderedSetEntry orderedSetEntry = members.remove(member);
     if (orderedSetEntry != null) {
+      scoreSet.remove(orderedSetEntry);
       oldValue = orderedSetEntry.getScoreBytes();
       sizeInBytes -= calculateSizeOfFieldValuePair(member, oldValue);
     }
diff --git a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
index 5537cbe..f59058f 100644
--- a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
+++ b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/data/RedisSortedSetTest.java
@@ -322,7 +322,6 @@ public class RedisSortedSetTest {
     assertThat(rangeList).isEmpty();
   }
 
-
   @Test
   public void zrange_ShouldReturnSimpleRanges() {
     Collection<byte[]> rangeList = rangeSortedSet.zrange(0, 5, false);
@@ -378,6 +377,16 @@ public class RedisSortedSetTest {
         "member6".getBytes(), "1.5".getBytes());
   }
 
+  @Test
+  public void scoreSet_shouldNotRetainOldEntries_whenEntriesUpdated() {
+    Collection<byte[]> rangeList = rangeSortedSet.zrange(0, 100, false);
+    assertThat(rangeList).hasSize(12);
+    assertThat(rangeList).containsExactly("member1".getBytes(), "1.0".getBytes(),
+        "member2".getBytes(), "1.1".getBytes(), "member3".getBytes(), "1.2".getBytes(),
+        "member4".getBytes(), "1.3".getBytes(), "member5".getBytes(), "1.4".getBytes(),
+        "member6".getBytes(), "1.5".getBytes());
+  }
+
   private RedisSortedSet createRedisSortedSet(String... membersAndScores) {
     final List<byte[]> membersAndScoresList = Arrays
         .stream(membersAndScores)

[geode] 01/03: Clean up unused code

Posted by ri...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ringles pushed a commit to branch GEODE-9375-Implement-ZRANGE-Radish-command
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 8a0f35fc9e9fee8994c3131ccde6e52923a44a18
Author: Ray Ingles <ri...@vmware.com>
AuthorDate: Thu Jul 1 10:44:43 2021 -0400

    Clean up unused code
---
 .../internal/collections/OrderStatisticsTree.java  | 33 ----------------------
 1 file changed, 33 deletions(-)

diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/OrderStatisticsTree.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/OrderStatisticsTree.java
index b619136..3be9e7d 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/OrderStatisticsTree.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/OrderStatisticsTree.java
@@ -215,11 +215,6 @@ public class OrderStatisticsTree<E extends Comparable<? super E>>
     return entryList;
   }
 
-  public Iterator<E> iterator(int index) {
-    Node<E> node = getNode(index);
-    return new TreeIterator(node);
-  }
-
   private Node<E> getNode(int index) {
     checkIndex(index);
     Node<E> node = root;
@@ -236,26 +231,6 @@ public class OrderStatisticsTree<E extends Comparable<? super E>>
     }
   }
 
-  private Node<E> findMinNode(E o) {
-    Node<E> x = root;
-    int cmp;
-
-    while ((cmp = o.compareTo(x.key)) != 0) {
-      if (cmp < 0) {
-        if (x.left == null) {
-          break;
-        }
-        x = x.left;
-      } else {
-        if (x.right == null) {
-          break;
-        }
-        x = x.right;
-      }
-    }
-    return x;
-  }
-
   @Override
   @SuppressWarnings("unchecked")
   public boolean contains(Object o) {
@@ -776,14 +751,6 @@ public class OrderStatisticsTree<E extends Comparable<? super E>>
       }
     }
 
-    public TreeIterator(Node<E> node) {
-      if (root == null) {
-        nextNode = null;
-      } else {
-        nextNode = node;
-      }
-    }
-
     @Override
     public boolean hasNext() {
       return nextNode != null;

[geode] 02/03: Clean up unused code

Posted by ri...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ringles pushed a commit to branch GEODE-9375-Implement-ZRANGE-Radish-command
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 515a47e941cbd79e5ac683fa9e04794e1f5b0c2b
Author: Ray Ingles <ri...@vmware.com>
AuthorDate: Thu Jul 1 11:07:50 2021 -0400

    Clean up unused code
---
 .../org/apache/geode/redis/internal/collections/IndexibleTreeSet.java | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/IndexibleTreeSet.java b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/IndexibleTreeSet.java
index d89e04a..dd66c96 100644
--- a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/IndexibleTreeSet.java
+++ b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/collections/IndexibleTreeSet.java
@@ -54,8 +54,4 @@ class IndexibleTreeSet<E> extends TreeSet<E> implements OrderStatisticsSet<E> {
   public ArrayList<E> getIndexRange(int min, int max) {
     return null;
   }
-
-  public int findMin(E element) {
-    return 0;
-  }
 }