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 <[email protected]> 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
