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 2022/02/15 18:23:28 UTC

[geode] 01/01: Ensure lpush atomic

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

ringles pushed a commit to branch WIP-GEODE-9892
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 2a3713f1caf6655897c720c2be0791a4e098875a
Author: Ray Ingles <ri...@vmware.com>
AuthorDate: Tue Feb 15 13:21:25 2022 -0500

    Ensure lpush atomic
---
 .../commands/executor/list/AbstractLPopIntegrationTest.java    | 10 +++++-----
 .../commands/executor/list/AbstractLPushIntegrationTest.java   | 10 ++++++++++
 .../apache/geode/redis/internal/commands/RedisCommandType.java |  3 ++-
 .../java/org/apache/geode/redis/internal/data/RedisList.java   |  7 +++----
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPopIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPopIntegrationTest.java
index fd7979d..3457635 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPopIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPopIntegrationTest.java
@@ -96,8 +96,8 @@ public abstract class AbstractLPopIntegrationTest implements RedisIntegrationTes
 
   @Test
   public void lpop_withConcurrentLPush_returnsCorrectValue() {
-    String[] valuesInitial = new String[] {"one", "two", "three"};
-    String[] valuesToAdd = new String[] {"pear", "apple", "plum", "orange", "peach"};
+    String[] valuesInitial = new String[] {"un", "deux", "troix"};
+    String[] valuesToAdd = new String[] {"plum", "peach", "orange"};
     jedis.lpush(KEY, valuesInitial);
 
     final AtomicReference<String> lpopReference = new AtomicReference<>();
@@ -106,9 +106,9 @@ public abstract class AbstractLPopIntegrationTest implements RedisIntegrationTes
         i -> lpopReference.set(jedis.lpop(KEY)))
             .runWithAction(() -> {
               assertThat(lpopReference).satisfiesAnyOf(
-                  lpopResult -> assertThat(lpopReference.get()).isEqualTo("peach"),
-                  lpopResult -> assertThat(lpopReference.get()).isEqualTo("three"),
-                  lpopResult -> assertThat(lpopResult.get()).isNull());
+                  lpopResult -> assertThat(lpopReference.get()).isEqualTo("orange"),
+                  lpopResult -> assertThat(lpopReference.get()).isEqualTo("troix"),
+                  lpopResult -> assertThat(lpopReference.get()).isNull());
               jedis.del(KEY);
               jedis.lpush(KEY, valuesInitial);
             });
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPushIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPushIntegrationTest.java
index f3ecf43..25959fd 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPushIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/list/AbstractLPushIntegrationTest.java
@@ -95,8 +95,18 @@ public abstract class AbstractLPushIntegrationTest implements RedisIntegrationTe
   @Test
   public void lpush_addsElementsInCorrectOrder_givenMultipleElements() {
     jedis.lpush(KEY, "e1", "e2", "e3");
+    jedis.lpush(KEY, "e4", "e5", "e6");
 
     String result = jedis.lpop(KEY);
+    assertThat(result).isEqualTo("e6");
+
+    result = jedis.lpop(KEY);
+    assertThat(result).isEqualTo("e5");
+
+    result = jedis.lpop(KEY);
+    assertThat(result).isEqualTo("e4");
+
+    result = jedis.lpop(KEY);
     assertThat(result).isEqualTo("e3");
 
     result = jedis.lpop(KEY);
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
index e5ea743..d67a7a1 100755
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
@@ -382,7 +382,8 @@ public enum RedisCommandType {
 
   LLEN(new LLenExecutor(), Category.LIST, SUPPORTED, new Parameter().exact(2).flags(READONLY, FAST)),
   LPOP(new LPopExecutor(), Category.LIST, SUPPORTED, new Parameter().exact(2).flags(WRITE, FAST)),
-  LPUSH(new LPushExecutor(), Category.LIST, SUPPORTED, new Parameter().min(3).flags(WRITE, DENYOOM, FAST)),
+  LPUSH(new LPushExecutor(), Category.LIST, SUPPORTED,
+      new Parameter().min(3).flags(WRITE, DENYOOM, FAST)),
 
   /********** Publish Subscribe **********/
 
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
index 61f9b79..02928d0 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
@@ -49,9 +49,6 @@ public class RedisList extends AbstractRedisData {
    * @return the number of elements actually added
    */
   public long lpush(List<byte[]> elementsToAdd, Region<RedisKey, RedisData> region, RedisKey key) {
-    // for (byte[] element : elementsToAdd) {
-    // elementPush(element);
-    // }
     elementsPush(elementsToAdd);
     storeChanges(region, key, new AddByteArrays(elementsToAdd));
     return elementList.size();
@@ -133,7 +130,9 @@ public class RedisList extends AbstractRedisData {
   }
 
   public synchronized void elementsPush(List<byte[]> elementsToAdd) {
-    elementList.addAll(0, elementsToAdd);
+    for (byte[] element : elementsToAdd) {
+      elementPush(element);
+    }
   }
 
   @Override