This is an automated email from the ASF dual-hosted git repository. upthewaterspout pushed a commit to branch feature/redis-performance-testing in repository https://gitbox.apache.org/repos/asf/geode.git
commit 64b7c208bab3489dfb065a05c372a5811650fc96 Author: Dan Smith <[email protected]> AuthorDate: Fri Mar 12 13:14:02 2021 -0800 Removing ByteArrayWrapper from hashes Also removing a couple of extra unneeded allocations during logging and message parsing. Moving the hash commands executor to a shared object rather than allocating it for each operation. --- geode-redis/build.gradle | 1 + .../redis/internal/data/AbstractRedisData.java | 2 +- .../geode/redis/internal/data/NullRedisHash.java | 10 +- .../geode/redis/internal/data/RedisHash.java | 137 ++++++++++++--------- .../data/RedisHashCommandsFunctionExecutor.java | 24 ++-- .../apache/geode/redis/internal/data/RedisSet.java | 14 ++- .../geode/redis/internal/delta/AddsDeltaInfo.java | 12 +- .../geode/redis/internal/delta/RemsDeltaInfo.java | 9 +- .../redis/internal/executor/CommandFunction.java | 16 +-- .../redis/internal/executor/RedisResponse.java | 2 +- .../redis/internal/executor/hash/HDelExecutor.java | 6 +- .../internal/executor/hash/HExistsExecutor.java | 5 +- .../internal/executor/hash/HGetAllExecutor.java | 4 +- .../redis/internal/executor/hash/HGetExecutor.java | 7 +- .../internal/executor/hash/HIncrByExecutor.java | 5 +- .../executor/hash/HIncrByFloatExecutor.java | 5 +- .../internal/executor/hash/HKeysExecutor.java | 4 +- .../redis/internal/executor/hash/HLenExecutor.java | 2 +- .../internal/executor/hash/HMGetExecutor.java | 8 +- .../internal/executor/hash/HMSetExecutor.java | 7 +- .../internal/executor/hash/HScanExecutor.java | 2 +- .../redis/internal/executor/hash/HSetExecutor.java | 9 +- .../internal/executor/hash/HStrLenExecutor.java | 5 +- .../internal/executor/hash/HValsExecutor.java | 4 +- .../redis/internal/executor/hash/HashExecutor.java | 4 - .../internal/executor/hash/RedisHashCommands.java | 25 ++-- .../hash/RedisHashCommandsFunctionInvoker.java | 27 ++-- .../redis/internal/netty/ByteToCommandDecoder.java | 18 +-- .../apache/geode/redis/internal/netty/Coder.java | 2 +- .../internal/netty/ExecutionHandlerContext.java | 15 ++- .../geode/redis/internal/data/RedisHashTest.java | 20 +-- 31 files changed, 218 insertions(+), 193 deletions(-) diff --git a/geode-redis/build.gradle b/geode-redis/build.gradle index 8d0eb70..76cd000 100644 --- a/geode-redis/build.gradle +++ b/geode-redis/build.gradle @@ -35,6 +35,7 @@ dependencies { implementation(project(':geode-logging')) implementation(project(':geode-core')) implementation(project(':geode-gfsh')) + implementation('it.unimi.dsi:fastutil') implementation('com.github.davidmoten:geo') implementation('io.netty:netty-all') implementation('org.apache.logging.log4j:log4j-api') diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/AbstractRedisData.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/AbstractRedisData.java index 93a4c01..46b41cb 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/AbstractRedisData.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/AbstractRedisData.java @@ -203,7 +203,7 @@ public abstract class AbstractRedisData implements RedisData { } } - private ArrayList<ByteArrayWrapper> readArrayList(DataInput in) throws IOException { + private <T> ArrayList<T> readArrayList(DataInput in) throws IOException { try { return DataSerializer.readArrayList(in); } catch (ClassNotFoundException e) { diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java index 3a453ec..5ead448 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisHash.java @@ -37,26 +37,26 @@ public class NullRedisHash extends RedisHash { @Override public int hset(Region<RedisKey, RedisData> region, RedisKey key, - List<ByteArrayWrapper> fieldsToSet, boolean nx) { + List<byte[]> fieldsToSet, boolean nx) { region.put(key, new RedisHash(fieldsToSet)); return fieldsToSet.size() / 2; } @Override public long hincrby(Region<RedisKey, RedisData> region, RedisKey key, - ByteArrayWrapper field, long increment) + byte[] field, long increment) throws NumberFormatException, ArithmeticException { region.put(key, - new RedisHash(Arrays.asList(field, new ByteArrayWrapper(Coder.longToBytes(increment))))); + new RedisHash(Arrays.asList(field, Coder.longToBytes(increment)))); return increment; } @Override public BigDecimal hincrbyfloat(Region<RedisKey, RedisData> region, RedisKey key, - ByteArrayWrapper field, BigDecimal increment) throws NumberFormatException { + byte[] field, BigDecimal increment) throws NumberFormatException { region.put(key, new RedisHash( - Arrays.asList(field, new ByteArrayWrapper(Coder.bigDecimalToBytes(increment))))); + Arrays.asList(field, Coder.bigDecimalToBytes(increment)))); return increment; } } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java index d415d3a..77f7583 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHash.java @@ -25,14 +25,16 @@ import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; +import it.unimi.dsi.fastutil.bytes.ByteArrays; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; @@ -45,11 +47,11 @@ import org.apache.geode.redis.internal.netty.Coder; public class RedisHash extends AbstractRedisData { public static final RedisHash NULL_REDIS_HASH = new NullRedisHash(); - private HashMap<ByteArrayWrapper, ByteArrayWrapper> hash; + private Object2ObjectOpenCustomHashMap<byte[], byte[]> hash; - public RedisHash(List<ByteArrayWrapper> fieldsToSet) { - hash = new HashMap<>(); - Iterator<ByteArrayWrapper> iterator = fieldsToSet.iterator(); + public RedisHash(List<byte[]> fieldsToSet) { + hash = new Object2ObjectOpenCustomHashMap<>(fieldsToSet.size(), ByteArrays.HASH_STRATEGY); + Iterator<byte[]> iterator = fieldsToSet.iterator(); while (iterator.hasNext()) { hashPut(iterator.next(), iterator.next()); } @@ -67,54 +69,64 @@ public class RedisHash extends AbstractRedisData { @Override public synchronized void toData(DataOutput out) throws IOException { super.toData(out); - DataSerializer.writeHashMap(hash, out); + DataSerializer.writePrimitiveInt(hash.size(), out); + for (Map.Entry<byte[], byte[]> entry : hash.entrySet()) { + byte[] key = entry.getKey(); + byte[] value = entry.getValue(); + DataSerializer.writeByteArray(key, out); + DataSerializer.writeByteArray(value, out); + } } - private synchronized ByteArrayWrapper hashPut(ByteArrayWrapper field, ByteArrayWrapper value) { + private synchronized byte[] hashPut(byte[] field, byte[] value) { return hash.put(field, value); } - private synchronized ByteArrayWrapper hashPutIfAbsent(ByteArrayWrapper field, - ByteArrayWrapper value) { + private synchronized byte[] hashPutIfAbsent(byte[] field, + byte[] value) { return hash.putIfAbsent(field, value); } - private synchronized ByteArrayWrapper hashRemove(ByteArrayWrapper field) { + private synchronized byte[] hashRemove(byte[] field) { return hash.remove(field); } @Override public void fromData(DataInput in) throws IOException, ClassNotFoundException { super.fromData(in); - hash = DataSerializer.readHashMap(in); + int size = DataSerializer.readInteger(in); + hash = new Object2ObjectOpenCustomHashMap<>(size, ByteArrays.HASH_STRATEGY); + for(int i = 0; i < size; i++) { + hash.put(DataSerializer.readByteArray(in), DataSerializer.readByteArray(in)); + } } @Override protected void applyDelta(DeltaInfo deltaInfo) { if (deltaInfo instanceof AddsDeltaInfo) { AddsDeltaInfo addsDeltaInfo = (AddsDeltaInfo) deltaInfo; - Iterator<ByteArrayWrapper> iterator = addsDeltaInfo.getAdds().iterator(); + Iterator<byte[]> iterator = addsDeltaInfo.getAdds().iterator(); while (iterator.hasNext()) { - ByteArrayWrapper field = iterator.next(); - ByteArrayWrapper value = iterator.next(); + byte[] field = iterator.next(); + byte[] value = iterator.next(); hashPut(field, value); } } else { RemsDeltaInfo remsDeltaInfo = (RemsDeltaInfo) deltaInfo; - for (ByteArrayWrapper field : remsDeltaInfo.getRemoves()) { + for (byte[] field : remsDeltaInfo.getRemoves()) { hashRemove(field); } } } public int hset(Region<RedisKey, RedisData> region, RedisKey key, - List<ByteArrayWrapper> fieldsToSet, boolean nx) { + List<byte[]> fieldsToSet, boolean nx) { int fieldsAdded = 0; AddsDeltaInfo deltaInfo = null; - Iterator<ByteArrayWrapper> iterator = fieldsToSet.iterator(); + Iterator<byte[]> iterator = fieldsToSet.iterator(); while (iterator.hasNext()) { - ByteArrayWrapper field = iterator.next(); - ByteArrayWrapper value = iterator.next(); + byte[] field = iterator.next(); + byte[] value = iterator.next(); boolean added = true; boolean newField; if (nx) { @@ -126,7 +138,7 @@ public class RedisHash extends AbstractRedisData { if (added) { if (deltaInfo == null) { - deltaInfo = new AddsDeltaInfo(); + deltaInfo = new AddsDeltaInfo(fieldsToSet.size()); } deltaInfo.add(field); deltaInfo.add(value); @@ -141,10 +153,10 @@ public class RedisHash extends AbstractRedisData { } public int hdel(Region<RedisKey, RedisData> region, RedisKey key, - List<ByteArrayWrapper> fieldsToRemove) { + List<byte[]> fieldsToRemove) { int fieldsRemoved = 0; RemsDeltaInfo deltaInfo = null; - for (ByteArrayWrapper fieldToRemove : fieldsToRemove) { + for (byte[] fieldToRemove : fieldsToRemove) { if (hashRemove(fieldToRemove) != null) { if (deltaInfo == null) { deltaInfo = new RemsDeltaInfo(); @@ -157,16 +169,16 @@ public class RedisHash extends AbstractRedisData { return fieldsRemoved; } - public Collection<ByteArrayWrapper> hgetall() { - ArrayList<ByteArrayWrapper> result = new ArrayList<>(); - for (Map.Entry<ByteArrayWrapper, ByteArrayWrapper> entry : hash.entrySet()) { + public Collection<byte[]> hgetall() { + ArrayList<byte[]> result = new ArrayList<>(hash.size()); + for (Map.Entry<byte[], byte[]> entry : hash.entrySet()) { result.add(entry.getKey()); result.add(entry.getValue()); } return result; } - public int hexists(ByteArrayWrapper field) { + public int hexists(byte[] field) { if (hash.containsKey(field)) { return 1; } else { @@ -174,7 +186,7 @@ public class RedisHash extends AbstractRedisData { } } - public ByteArrayWrapper hget(ByteArrayWrapper field) { + public byte[] hget(byte[] field) { return hash.get(field); } @@ -182,36 +194,36 @@ public class RedisHash extends AbstractRedisData { return hash.size(); } - public int hstrlen(ByteArrayWrapper field) { - ByteArrayWrapper entry = hget(field); - return entry != null ? entry.length() : 0; + public int hstrlen(byte[] field) { + byte[] entry = hget(field); + return entry != null ? entry.length : 0; } - public List<ByteArrayWrapper> hmget(List<ByteArrayWrapper> fields) { - ArrayList<ByteArrayWrapper> results = new ArrayList<>(fields.size()); - for (ByteArrayWrapper field : fields) { + public List<byte[]> hmget(List<byte[]> fields) { + ArrayList<byte[]> results = new ArrayList<>(fields.size()); + for (byte[] field : fields) { results.add(hash.get(field)); } return results; } - public Collection<ByteArrayWrapper> hvals() { + public Collection<byte[]> hvals() { return new ArrayList<>(hash.values()); } - public Collection<ByteArrayWrapper> hkeys() { + public Collection<byte[]> hkeys() { return new ArrayList<>(hash.keySet()); } - public Pair<BigInteger, List<Object>> hscan(Pattern matchPattern, int count, BigInteger cursor) { - List<Object> returnList = new ArrayList<Object>(); + public Pair<BigInteger, List<byte[]>> hscan(Pattern matchPattern, int count, BigInteger cursor) { + List<byte[]> returnList = new ArrayList<>(); int size = hash.size(); BigInteger beforeCursor = new BigInteger("0"); int numElements = 0; int i = -1; - for (Map.Entry<ByteArrayWrapper, ByteArrayWrapper> entry : hash.entrySet()) { - ByteArrayWrapper key = entry.getKey(); - ByteArrayWrapper value = entry.getValue(); + for (Map.Entry<byte[], byte[]> entry : hash.entrySet()) { + byte[] key = entry.getKey(); + byte[] value = entry.getValue(); i++; if (beforeCursor.compareTo(cursor) < 0) { beforeCursor = beforeCursor.add(new BigInteger("1")); @@ -219,7 +231,7 @@ public class RedisHash extends AbstractRedisData { } if (matchPattern != null) { - if (matchPattern.matcher(key.toString()).matches()) { + if (matchPattern.matcher(Coder.bytesToString(key)).matches()) { returnList.add(key); returnList.add(value); numElements++; @@ -235,7 +247,7 @@ public class RedisHash extends AbstractRedisData { } } - Pair<BigInteger, List<Object>> scanResult; + Pair<BigInteger, List<byte[]>> scanResult; if (i >= size - 1) { scanResult = new ImmutablePair<>(new BigInteger("0"), returnList); } else { @@ -245,12 +257,12 @@ public class RedisHash extends AbstractRedisData { } public long hincrby(Region<RedisKey, RedisData> region, RedisKey key, - ByteArrayWrapper field, long increment) throws NumberFormatException, ArithmeticException { - ByteArrayWrapper oldValue = hash.get(field); + byte[] field, long increment) throws NumberFormatException, ArithmeticException { + byte[] oldValue = hash.get(field); if (oldValue == null) { - ByteArrayWrapper newValue = new ByteArrayWrapper(Coder.longToBytes(increment)); + byte[] newValue = Coder.longToBytes(increment); hashPut(field, newValue); - AddsDeltaInfo deltaInfo = new AddsDeltaInfo(); + AddsDeltaInfo deltaInfo = new AddsDeltaInfo(2); deltaInfo.add(field); deltaInfo.add(newValue); storeChanges(region, key, deltaInfo); @@ -259,7 +271,7 @@ public class RedisHash extends AbstractRedisData { long value; try { - value = Long.parseLong(oldValue.toString()); + value = Long.parseLong(Coder.bytesToString(oldValue)); } catch (NumberFormatException ex) { throw new NumberFormatException(ERROR_NOT_INTEGER); } @@ -270,9 +282,9 @@ public class RedisHash extends AbstractRedisData { value += increment; - ByteArrayWrapper modifiedValue = new ByteArrayWrapper(Coder.longToBytes(value)); + byte[] modifiedValue = Coder.longToBytes(value); hashPut(field, modifiedValue); - AddsDeltaInfo deltaInfo = new AddsDeltaInfo(); + AddsDeltaInfo deltaInfo = new AddsDeltaInfo(2); deltaInfo.add(field); deltaInfo.add(modifiedValue); storeChanges(region, key, deltaInfo); @@ -280,19 +292,19 @@ public class RedisHash extends AbstractRedisData { } public BigDecimal hincrbyfloat(Region<RedisKey, RedisData> region, RedisKey key, - ByteArrayWrapper field, BigDecimal increment) throws NumberFormatException { - ByteArrayWrapper oldValue = hash.get(field); + byte[] field, BigDecimal increment) throws NumberFormatException { + byte[] oldValue = hash.get(field); if (oldValue == null) { - ByteArrayWrapper newValue = new ByteArrayWrapper(Coder.bigDecimalToBytes(increment)); + byte[] newValue = Coder.bigDecimalToBytes(increment); hashPut(field, newValue); - AddsDeltaInfo deltaInfo = new AddsDeltaInfo(); + AddsDeltaInfo deltaInfo = new AddsDeltaInfo(2); deltaInfo.add(field); deltaInfo.add(newValue); storeChanges(region, key, deltaInfo); return increment.stripTrailingZeros(); } - String valueS = oldValue.toString(); + String valueS = Coder.bytesToString(oldValue); if (valueS.contains(" ")) { throw new NumberFormatException("hash value is not a float"); } @@ -306,9 +318,9 @@ public class RedisHash extends AbstractRedisData { value = value.add(increment); - ByteArrayWrapper modifiedValue = new ByteArrayWrapper(Coder.bigDecimalToBytes(value)); + byte[] modifiedValue = Coder.bigDecimalToBytes(value); hashPut(field, modifiedValue); - AddsDeltaInfo deltaInfo = new AddsDeltaInfo(); + AddsDeltaInfo deltaInfo = new AddsDeltaInfo(2); deltaInfo.add(field); deltaInfo.add(modifiedValue); storeChanges(region, key, deltaInfo); @@ -337,7 +349,16 @@ public class RedisHash extends AbstractRedisData { return false; } RedisHash redisHash = (RedisHash) o; - return Objects.equals(hash, redisHash.hash); + if(hash.size() != redisHash.hash.size()) { + return false; + } + + for(Map.Entry<byte[], byte[]> entry : hash.entrySet()) { + if(!Arrays.equals(redisHash.hash.get(entry.getKey()), (entry.getValue()))) { + return false; + } + } + return true; } @Override @@ -347,6 +368,6 @@ public class RedisHash extends AbstractRedisData { @Override public String toString() { - return "RedisHash{" + super.toString() + ", " + "hash=" + hash + '}'; + return "RedisHash{" + super.toString() + ", " + "size=" + hash.size() + "}"; } } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java index 0f50e16..0c00668 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisHashCommandsFunctionExecutor.java @@ -39,31 +39,31 @@ public class RedisHashCommandsFunctionExecutor extends RedisDataCommandsFunction } @Override - public int hset(RedisKey key, List<ByteArrayWrapper> fieldsToSet, boolean NX) { + public int hset(RedisKey key, List<byte[]> fieldsToSet, boolean NX) { return stripedExecute(key, () -> getRedisHash(key, false) .hset(getRegion(), key, fieldsToSet, NX)); } @Override - public int hdel(RedisKey key, List<ByteArrayWrapper> fieldsToRemove) { + public int hdel(RedisKey key, List<byte[]> fieldsToRemove) { return stripedExecute(key, () -> getRedisHash(key, false) .hdel(getRegion(), key, fieldsToRemove)); } @Override - public Collection<ByteArrayWrapper> hgetall(RedisKey key) { + public Collection<byte[]> hgetall(RedisKey key) { return stripedExecute(key, () -> getRedisHash(key, true).hgetall()); } @Override - public int hexists(RedisKey key, ByteArrayWrapper field) { + public int hexists(RedisKey key, byte[] field) { return stripedExecute(key, () -> getRedisHash(key, true).hexists(field)); } @Override - public ByteArrayWrapper hget(RedisKey key, ByteArrayWrapper field) { + public byte[] hget(RedisKey key, byte[] field) { return stripedExecute(key, () -> getRedisHash(key, true).hget(field)); } @@ -73,27 +73,27 @@ public class RedisHashCommandsFunctionExecutor extends RedisDataCommandsFunction } @Override - public int hstrlen(RedisKey key, ByteArrayWrapper field) { + public int hstrlen(RedisKey key, byte[] field) { return stripedExecute(key, () -> getRedisHash(key, true).hstrlen(field)); } @Override - public List<ByteArrayWrapper> hmget(RedisKey key, List<ByteArrayWrapper> fields) { + public List<byte[]> hmget(RedisKey key, List<byte[]> fields) { return stripedExecute(key, () -> getRedisHash(key, true).hmget(fields)); } @Override - public Collection<ByteArrayWrapper> hvals(RedisKey key) { + public Collection<byte[]> hvals(RedisKey key) { return stripedExecute(key, () -> getRedisHash(key, true).hvals()); } @Override - public Collection<ByteArrayWrapper> hkeys(RedisKey key) { + public Collection<byte[]> hkeys(RedisKey key) { return stripedExecute(key, () -> getRedisHash(key, true).hkeys()); } @Override - public Pair<BigInteger, List<Object>> hscan(RedisKey key, Pattern matchPattern, + public Pair<BigInteger, List<byte[]>> hscan(RedisKey key, Pattern matchPattern, int count, BigInteger cursor) { return stripedExecute(key, () -> getRedisHash(key, true) @@ -101,14 +101,14 @@ public class RedisHashCommandsFunctionExecutor extends RedisDataCommandsFunction } @Override - public long hincrby(RedisKey key, ByteArrayWrapper field, long increment) { + public long hincrby(RedisKey key, byte[] field, long increment) { return stripedExecute(key, () -> getRedisHash(key, false) .hincrby(getRegion(), key, field, increment)); } @Override - public BigDecimal hincrbyfloat(RedisKey key, ByteArrayWrapper field, BigDecimal increment) { + public BigDecimal hincrbyfloat(RedisKey key, byte[] field, BigDecimal increment) { return stripedExecute(key, () -> getRedisHash(key, false) .hincrbyfloat(getRegion(), key, field, increment)); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java index d5d745d..25286a2 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java @@ -31,6 +31,8 @@ import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; @@ -122,7 +124,8 @@ public class RedisSet extends AbstractRedisData { } } if (!popped.isEmpty()) { - storeChanges(region, key, new RemsDeltaInfo(popped)); + storeChanges(region, key, new RemsDeltaInfo(popped.stream().map(ByteArrayWrapper::toBytes).collect( + Collectors.toCollection(ArrayList::new)))); } return popped; } @@ -207,7 +210,8 @@ public class RedisSet extends AbstractRedisData { } private synchronized boolean membersAddAll(AddsDeltaInfo addsDeltaInfo) { - return members.addAll(addsDeltaInfo.getAdds()); + return members.addAll(addsDeltaInfo.getAdds().stream().map(ByteArrayWrapper::new).collect( + Collectors.toList())); } private synchronized boolean membersRemoveAll(RemsDeltaInfo remsDeltaInfo) { @@ -233,7 +237,8 @@ public class RedisSet extends AbstractRedisData { membersToAdd.removeIf(memberToAdd -> !membersAdd(memberToAdd)); int membersAdded = membersToAdd.size(); if (membersAdded != 0) { - storeChanges(region, key, new AddsDeltaInfo(membersToAdd)); + final ArrayList<byte[]> rStream = membersToAdd.stream().map(ByteArrayWrapper::toBytes).collect(Collectors.toCollection(() -> new ArrayList<>())); + storeChanges(region, key, new AddsDeltaInfo(rStream)); } return membersAdded; } @@ -251,7 +256,8 @@ public class RedisSet extends AbstractRedisData { membersToRemove.removeIf(memberToRemove -> !membersRemove(memberToRemove)); int membersRemoved = membersToRemove.size(); if (membersRemoved != 0) { - storeChanges(region, key, new RemsDeltaInfo(membersToRemove)); + storeChanges(region, key, new RemsDeltaInfo(membersToRemove.stream().map(ByteArrayWrapper::toBytes).collect( + Collectors.toCollection(ArrayList::new)))); } return membersRemoved; } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/AddsDeltaInfo.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/AddsDeltaInfo.java index 03f1787..1cef973 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/AddsDeltaInfo.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/AddsDeltaInfo.java @@ -24,17 +24,17 @@ import org.apache.geode.DataSerializer; import org.apache.geode.redis.internal.data.ByteArrayWrapper; public class AddsDeltaInfo implements DeltaInfo { - private final ArrayList<ByteArrayWrapper> deltas; + private final ArrayList<byte[]> deltas; - public AddsDeltaInfo() { - this(new ArrayList<>()); + public AddsDeltaInfo(int size) { + this(new ArrayList<>(size)); } - public AddsDeltaInfo(ArrayList<ByteArrayWrapper> deltas) { + public AddsDeltaInfo(ArrayList<byte[]> deltas) { this.deltas = deltas; } - public void add(ByteArrayWrapper delta) { + public void add(byte[] delta) { deltas.add(delta); } @@ -43,7 +43,7 @@ public class AddsDeltaInfo implements DeltaInfo { DataSerializer.writeArrayList(deltas, out); } - public ArrayList<ByteArrayWrapper> getAdds() { + public ArrayList<byte[]> getAdds() { return deltas; } } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/RemsDeltaInfo.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/RemsDeltaInfo.java index fe331ed..1038f9d 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/RemsDeltaInfo.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/delta/RemsDeltaInfo.java @@ -21,20 +21,19 @@ import java.io.IOException; import java.util.ArrayList; import org.apache.geode.DataSerializer; -import org.apache.geode.redis.internal.data.ByteArrayWrapper; public class RemsDeltaInfo implements DeltaInfo { - private final ArrayList<ByteArrayWrapper> deltas; + private final ArrayList<byte[]> deltas; public RemsDeltaInfo() { this(new ArrayList<>()); } - public RemsDeltaInfo(ArrayList<ByteArrayWrapper> deltas) { + public RemsDeltaInfo(ArrayList<byte[]> deltas) { this.deltas = deltas; } - public void add(ByteArrayWrapper delta) { + public void add(byte[] delta) { deltas.add(delta); } @@ -43,7 +42,7 @@ public class RemsDeltaInfo implements DeltaInfo { DataSerializer.writeArrayList(deltas, out); } - public ArrayList<ByteArrayWrapper> getRemoves() { + public ArrayList<byte[]> getRemoves() { return deltas; } } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java index 17d3c5a..7bda1f9 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/CommandFunction.java @@ -223,32 +223,32 @@ public class CommandFunction extends SingleResultRedisFunction { return setCommands.sdiffstore(key, setKeys); } case HSET: { - List<ByteArrayWrapper> fieldsToSet = (List<ByteArrayWrapper>) args[1]; + List<byte[]> fieldsToSet = (List<byte[]>) args[1]; boolean NX = (boolean) args[2]; return hashCommands.hset(key, fieldsToSet, NX); } case HDEL: { - List<ByteArrayWrapper> fieldsToRemove = (List<ByteArrayWrapper>) args[1]; + List<byte[]> fieldsToRemove = (List<byte[]>) args[1]; return hashCommands.hdel(key, fieldsToRemove); } case HGETALL: return hashCommands.hgetall(key); case HEXISTS: { - ByteArrayWrapper field = (ByteArrayWrapper) args[1]; + byte[] field = (byte[]) args[1]; return hashCommands.hexists(key, field); } case HGET: { - ByteArrayWrapper field = (ByteArrayWrapper) args[1]; + byte[] field = (byte[]) args[1]; return hashCommands.hget(key, field); } case HLEN: return hashCommands.hlen(key); case HSTRLEN: { - ByteArrayWrapper field = (ByteArrayWrapper) args[1]; + byte[] field = (byte[]) args[1]; return hashCommands.hstrlen(key, field); } case HMGET: { - List<ByteArrayWrapper> fields = (List<ByteArrayWrapper>) args[1]; + List<byte[]> fields = (List<byte[]>) args[1]; return hashCommands.hmget(key, fields); } case HVALS: @@ -262,12 +262,12 @@ public class CommandFunction extends SingleResultRedisFunction { return hashCommands.hscan(key, pattern, count, cursor); } case HINCRBY: { - ByteArrayWrapper field = (ByteArrayWrapper) args[1]; + byte[] field = (byte[]) args[1]; long increment = (long) args[2]; return hashCommands.hincrby(key, field, increment); } case HINCRBYFLOAT: { - ByteArrayWrapper field = (ByteArrayWrapper) args[1]; + byte[] field = (byte[]) args[1]; BigDecimal increment = (BigDecimal) args[2]; return hashCommands.hincrbyfloat(key, field, increment); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/RedisResponse.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/RedisResponse.java index f6e909e..5b94574 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/RedisResponse.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/RedisResponse.java @@ -129,7 +129,7 @@ public class RedisResponse { return new RedisResponse((buffer) -> Coder.getWrongTypeResponse(buffer, error)); } - public static RedisResponse scan(BigInteger cursor, List<Object> scanResult) { + public static RedisResponse scan(BigInteger cursor, List<?> scanResult) { return new RedisResponse((buffer) -> Coder.getScanResponse(buffer, cursor, scanResult)); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HDelExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HDelExecutor.java index e259b47..ab33beb 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HDelExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HDelExecutor.java @@ -45,11 +45,11 @@ public class HDelExecutor extends HashExecutor { @Override public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { - List<ByteArrayWrapper> commandElems = command.getProcessedCommandWrappers(); + List<byte[]> commandElems = command.getProcessedCommand(); RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - ArrayList<ByteArrayWrapper> fieldsToDelete = + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + ArrayList<byte[]> fieldsToDelete = new ArrayList<>(commandElems.subList(2, commandElems.size())); int numDeleted = redisHashCommands.hdel(key, fieldsToDelete); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HExistsExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HExistsExecutor.java index a2d6a25..5ff89fe 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HExistsExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HExistsExecutor.java @@ -45,10 +45,9 @@ public class HExistsExecutor extends HashExecutor { ExecutionHandlerContext context) { List<byte[]> commandElems = command.getProcessedCommand(); - byte[] byteField = commandElems.get(FIELD_INDEX); - ByteArrayWrapper field = new ByteArrayWrapper(byteField); + byte[] field = commandElems.get(FIELD_INDEX); RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); return RedisResponse.integer(redisHashCommands.hexists(key, field)); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetAllExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetAllExecutor.java index c0cc768..6f214fd 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetAllExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetAllExecutor.java @@ -47,8 +47,8 @@ public class HGetAllExecutor extends HashExecutor { public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - Collection<ByteArrayWrapper> fieldsAndValues = redisHashCommands.hgetall(key); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + Collection<byte[]> fieldsAndValues = redisHashCommands.hgetall(key); return RedisResponse.array(fieldsAndValues); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetExecutor.java index 7bb5644..0fe626b 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HGetExecutor.java @@ -42,11 +42,10 @@ public class HGetExecutor extends HashExecutor { ExecutionHandlerContext context) { List<byte[]> commandElems = command.getProcessedCommand(); - byte[] byteField = commandElems.get(FIELD_INDEX); - ByteArrayWrapper field = new ByteArrayWrapper(byteField); + byte[] field = commandElems.get(FIELD_INDEX); RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - ByteArrayWrapper valueWrapper = redisHashCommands.hget(key, field); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + byte[] valueWrapper = redisHashCommands.hget(key, field); if (valueWrapper != null) { return RedisResponse.bulkString(valueWrapper); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByExecutor.java index ceee123..8283228 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByExecutor.java @@ -55,8 +55,7 @@ public class HIncrByExecutor extends HashExecutor { ExecutionHandlerContext context) { List<byte[]> commandElems = command.getProcessedCommand(); RedisKey key = command.getKey(); - byte[] byteField = commandElems.get(FIELD_INDEX); - ByteArrayWrapper field = new ByteArrayWrapper(byteField); + byte[] field = commandElems.get(FIELD_INDEX); byte[] incrArray = commandElems.get(INCREMENT_INDEX); long increment; @@ -66,7 +65,7 @@ public class HIncrByExecutor extends HashExecutor { return RedisResponse.error(ERROR_INCREMENT_NOT_USABLE); } - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); long value = redisHashCommands.hincrby(key, field, increment); return RedisResponse.integer(value); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java index 148ee41..d6c69ef 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HIncrByFloatExecutor.java @@ -65,9 +65,8 @@ public class HIncrByFloatExecutor extends HashExecutor { } RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - byte[] byteField = commandElems.get(FIELD_INDEX); - ByteArrayWrapper field = new ByteArrayWrapper(byteField); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + byte[] field = commandElems.get(FIELD_INDEX); BigDecimal value = redisHashCommands.hincrbyfloat(key, field, validated.getLeft()); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HKeysExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HKeysExecutor.java index b54bb1f..783ad66 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HKeysExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HKeysExecutor.java @@ -47,8 +47,8 @@ public class HKeysExecutor extends HashExecutor { public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - Collection<ByteArrayWrapper> keys = redisHashCommands.hkeys(key); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + Collection<byte[]> keys = redisHashCommands.hkeys(key); if (keys.isEmpty()) { return RedisResponse.emptyArray(); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HLenExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HLenExecutor.java index 21a6390..e51d565 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HLenExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HLenExecutor.java @@ -39,7 +39,7 @@ public class HLenExecutor extends HashExecutor { public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); int len = redisHashCommands.hlen(key); return RedisResponse.integer(len); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMGetExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMGetExecutor.java index 09e3c63..aa96b4c8 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMGetExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMGetExecutor.java @@ -48,12 +48,12 @@ public class HMGetExecutor extends HashExecutor { ExecutionHandlerContext context) { RedisKey key = command.getKey(); - List<ByteArrayWrapper> commandElements = command.getProcessedCommandWrappers(); - ArrayList<ByteArrayWrapper> fields = + List<byte[]> commandElements = command.getProcessedCommand(); + ArrayList<byte[]> fields = new ArrayList<>(commandElements.subList(2, commandElements.size())); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); - List<ByteArrayWrapper> values = redisHashCommands.hmget(key, fields); + List<byte[]> values = redisHashCommands.hmget(key, fields); return RedisResponse.array(values); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMSetExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMSetExecutor.java index 7ce7cc0..a945e30 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMSetExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HMSetExecutor.java @@ -50,12 +50,11 @@ public class HMSetExecutor extends HashExecutor { @Override public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { - List<ByteArrayWrapper> commandElems = command.getProcessedCommandWrappers(); + List<byte[]> commandElems = command.getProcessedCommand(); RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - ArrayList<ByteArrayWrapper> fieldsToSet = - new ArrayList<>(commandElems.subList(2, commandElems.size())); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + List<byte[]> fieldsToSet = commandElems.subList(2, commandElems.size()); redisHashCommands.hset(key, fieldsToSet, false); return RedisResponse.ok(); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HScanExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HScanExecutor.java index 92d3a1a..e7a3b70 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HScanExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HScanExecutor.java @@ -113,7 +113,7 @@ public class HScanExecutor extends AbstractScanExecutor { RedisHashCommands redisHashCommands = new RedisHashCommandsFunctionInvoker(context.getRegionProvider().getDataRegion()); - Pair<BigInteger, List<Object>> scanResult = + Pair<BigInteger, List<byte[]>> scanResult = redisHashCommands.hscan(key, matchPattern, count, cursor); context.setHscanCursor(scanResult.getLeft()); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HSetExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HSetExecutor.java index 9823faf..bf833b2 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HSetExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HSetExecutor.java @@ -14,10 +14,8 @@ */ package org.apache.geode.redis.internal.executor.hash; -import java.util.ArrayList; import java.util.List; -import org.apache.geode.redis.internal.data.ByteArrayWrapper; import org.apache.geode.redis.internal.data.RedisKey; import org.apache.geode.redis.internal.executor.RedisResponse; import org.apache.geode.redis.internal.netty.Command; @@ -43,14 +41,13 @@ public class HSetExecutor extends HashExecutor { @Override public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { - List<ByteArrayWrapper> commandElems = command.getProcessedCommandWrappers(); + List<byte[]> commandElems = command.getProcessedCommand(); RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); - ArrayList<ByteArrayWrapper> fieldsToSet = - new ArrayList<>(commandElems.subList(2, commandElems.size())); + List<byte[]> fieldsToSet = commandElems.subList(2, commandElems.size()); int fieldsAdded = redisHashCommands.hset(key, fieldsToSet, onlySetOnAbsent()); return RedisResponse.integer(fieldsAdded); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HStrLenExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HStrLenExecutor.java index bdc8ba5..de192cd 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HStrLenExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HStrLenExecutor.java @@ -29,10 +29,9 @@ public class HStrLenExecutor extends HashExecutor { public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) { RedisKey key = command.getKey(); List<byte[]> commandElems = command.getProcessedCommand(); - byte[] byteField = commandElems.get(FIELD_INDEX); - ByteArrayWrapper field = new ByteArrayWrapper(byteField); + byte[] field = commandElems.get(FIELD_INDEX); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); int len = redisHashCommands.hstrlen(key, field); return RedisResponse.integer(len); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HValsExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HValsExecutor.java index 0156b6c..eccaca3 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HValsExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HValsExecutor.java @@ -54,8 +54,8 @@ public class HValsExecutor extends HashExecutor { ExecutionHandlerContext context) { RedisKey key = command.getKey(); - RedisHashCommands redisHashCommands = createRedisHashCommands(context); - Collection<ByteArrayWrapper> values = redisHashCommands.hvals(key); + RedisHashCommands redisHashCommands = context.getRedisHashCommands(); + Collection<byte[]> values = redisHashCommands.hvals(key); if (values.isEmpty()) { return RedisResponse.emptyArray(); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HashExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HashExecutor.java index ba2b9a7..3183693 100755 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HashExecutor.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HashExecutor.java @@ -24,8 +24,4 @@ import org.apache.geode.redis.internal.netty.ExecutionHandlerContext; public abstract class HashExecutor extends AbstractExecutor { static final int FIELD_INDEX = 2; - RedisHashCommands createRedisHashCommands(ExecutionHandlerContext context) { - return new RedisHashCommandsFunctionInvoker(context.getRegionProvider().getDataRegion()); - } - } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java index cc1319f..18bf5c7 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommands.java @@ -23,34 +23,33 @@ import java.util.regex.Pattern; import org.apache.commons.lang3.tuple.Pair; -import org.apache.geode.redis.internal.data.ByteArrayWrapper; import org.apache.geode.redis.internal.data.RedisKey; public interface RedisHashCommands { - int hset(RedisKey key, List<ByteArrayWrapper> fieldsToSet, boolean NX); + int hset(RedisKey key, List<byte[]> fieldsToSet, boolean NX); - int hdel(RedisKey key, List<ByteArrayWrapper> fieldsToRemove); + int hdel(RedisKey key, List<byte[]> fieldsToRemove); - Collection<ByteArrayWrapper> hgetall(RedisKey key); + Collection<byte[]> hgetall(RedisKey key); - int hexists(RedisKey key, ByteArrayWrapper field); + int hexists(RedisKey key, byte[] field); - ByteArrayWrapper hget(RedisKey key, ByteArrayWrapper field); + byte[] hget(RedisKey key, byte[] field); int hlen(RedisKey key); - int hstrlen(RedisKey key, ByteArrayWrapper field); + int hstrlen(RedisKey key, byte[] field); - List<ByteArrayWrapper> hmget(RedisKey key, List<ByteArrayWrapper> fields); + List<byte[]> hmget(RedisKey key, List<byte[]> fields); - Collection<ByteArrayWrapper> hvals(RedisKey key); + Collection<byte[]> hvals(RedisKey key); - Collection<ByteArrayWrapper> hkeys(RedisKey key); + Collection<byte[]> hkeys(RedisKey key); - Pair<BigInteger, List<Object>> hscan(RedisKey key, Pattern matchPattern, int count, + Pair<BigInteger, List<byte[]>> hscan(RedisKey key, Pattern matchPattern, int count, BigInteger cursor); - long hincrby(RedisKey key, ByteArrayWrapper field, long increment); + long hincrby(RedisKey key, byte[] field, long increment); - BigDecimal hincrbyfloat(RedisKey key, ByteArrayWrapper field, BigDecimal increment); + BigDecimal hincrbyfloat(RedisKey key, byte[] field, BigDecimal increment); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java index aeda9b6..cf7e806 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/RedisHashCommandsFunctionInvoker.java @@ -39,7 +39,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.FunctionService; -import org.apache.geode.redis.internal.data.ByteArrayWrapper; import org.apache.geode.redis.internal.data.RedisData; import org.apache.geode.redis.internal.data.RedisKey; import org.apache.geode.redis.internal.executor.CommandFunction; @@ -62,27 +61,27 @@ public class RedisHashCommandsFunctionInvoker extends RedisCommandsFunctionInvok } @Override - public int hset(RedisKey key, List<ByteArrayWrapper> fieldsToSet, boolean NX) { + public int hset(RedisKey key, List<byte[]> fieldsToSet, boolean NX) { return invokeCommandFunction(key, HSET, fieldsToSet, NX); } @Override - public int hdel(RedisKey key, List<ByteArrayWrapper> fieldsToRemove) { + public int hdel(RedisKey key, List<byte[]> fieldsToRemove) { return invokeCommandFunction(key, HDEL, fieldsToRemove); } @Override - public Collection<ByteArrayWrapper> hgetall(RedisKey key) { + public Collection<byte[]> hgetall(RedisKey key) { return invokeCommandFunction(key, HGETALL); } @Override - public int hexists(RedisKey key, ByteArrayWrapper field) { + public int hexists(RedisKey key, byte[] field) { return invokeCommandFunction(key, HEXISTS, field); } @Override - public ByteArrayWrapper hget(RedisKey key, ByteArrayWrapper field) { + public byte[] hget(RedisKey key, byte[] field) { return invokeCommandFunction(key, HGET, field); } @@ -92,39 +91,39 @@ public class RedisHashCommandsFunctionInvoker extends RedisCommandsFunctionInvok } @Override - public int hstrlen(RedisKey key, ByteArrayWrapper field) { + public int hstrlen(RedisKey key, byte[] field) { return invokeCommandFunction(key, HSTRLEN, field); } @Override - public List<ByteArrayWrapper> hmget(RedisKey key, - List<ByteArrayWrapper> fields) { + public List<byte[]> hmget(RedisKey key, + List<byte[]> fields) { return invokeCommandFunction(key, HMGET, fields); } @Override - public Collection<ByteArrayWrapper> hvals(RedisKey key) { + public Collection<byte[]> hvals(RedisKey key) { return invokeCommandFunction(key, HVALS); } @Override - public Collection<ByteArrayWrapper> hkeys(RedisKey key) { + public Collection<byte[]> hkeys(RedisKey key) { return invokeCommandFunction(key, HKEYS); } @Override - public Pair<BigInteger, List<Object>> hscan(RedisKey key, Pattern matchPattern, + public Pair<BigInteger, List<byte[]>> hscan(RedisKey key, Pattern matchPattern, int count, BigInteger cursor) { return invokeCommandFunction(key, HSCAN, matchPattern, count, cursor); } @Override - public long hincrby(RedisKey key, ByteArrayWrapper field, long increment) { + public long hincrby(RedisKey key, byte[] field, long increment) { return invokeCommandFunction(key, HINCRBY, field, increment); } @Override - public BigDecimal hincrbyfloat(RedisKey key, ByteArrayWrapper field, BigDecimal increment) { + public BigDecimal hincrbyfloat(RedisKey key, byte[] field, BigDecimal increment) { return invokeCommandFunction(key, HINCRBYFLOAT, field, increment); } diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ByteToCommandDecoder.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ByteToCommandDecoder.java index 4139daa..f59dfc4 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ByteToCommandDecoder.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ByteToCommandDecoder.java @@ -62,7 +62,7 @@ public class ByteToCommandDecoder extends ByteToMessageDecoder { @Override protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception { - Command c = null; + Command c; long bytesRead = 0; do { int startReadIndex = in.readerIndex(); @@ -90,35 +90,37 @@ public class ByteToCommandDecoder extends ByteToMessageDecoder { throw new RedisCommandParserException( "Expected: " + (char) arrayID + " Actual: " + (char) firstB); } - ArrayList<byte[]> commandElems = new ArrayList<byte[]>(); + List<byte[]> commandElems = parseArray(buffer); - if (!parseArray(commandElems, buffer)) { + if (commandElems == null) { return null; } return new Command(commandElems); } - private boolean parseArray(ArrayList<byte[]> commandElems, ByteBuf buffer) + private List<byte[]> parseArray(ByteBuf buffer) throws RedisCommandParserException { byte currentChar; int arrayLength = parseCurrentNumber(buffer); if (arrayLength == Integer.MIN_VALUE || !parseRN(buffer)) { - return false; + return null; } if (arrayLength < 0 || arrayLength > 1000000000) { throw new RedisCommandParserException("invalid multibulk length"); } + List<byte[]> commandElems = new ArrayList<>(arrayLength); + for (int i = 0; i < arrayLength; i++) { if (!buffer.isReadable()) { - return false; + return null; } currentChar = buffer.readByte(); if (currentChar == bulkStringID) { byte[] newBulkString = parseBulkString(buffer); if (newBulkString == null) { - return false; + return null; } commandElems.add(newBulkString); } else { @@ -126,7 +128,7 @@ public class ByteToCommandDecoder extends ByteToMessageDecoder { "expected: \'$\', got \'" + (char) currentChar + "\'"); } } - return true; + return commandElems; } /** diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java index e0a0e52..e90f508 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/Coder.java @@ -185,7 +185,7 @@ public class Coder { } public static ByteBuf getScanResponse(ByteBuf buffer, BigInteger cursor, - List<Object> scanResult) { + List<?> scanResult) { buffer.writeByte(ARRAY_ID); buffer.writeBytes(intToBytes(2)); buffer.writeBytes(CRLFar); diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java index 56e0cd8..c43905e 100644 --- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java +++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java @@ -18,6 +18,7 @@ package org.apache.geode.redis.internal.netty; import java.io.IOException; import java.math.BigInteger; +import java.net.SocketAddress; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; @@ -51,6 +52,9 @@ import org.apache.geode.redis.internal.RegionProvider; import org.apache.geode.redis.internal.data.RedisDataTypeMismatchException; import org.apache.geode.redis.internal.executor.CommandFunction; import org.apache.geode.redis.internal.executor.RedisResponse; +import org.apache.geode.redis.internal.executor.hash.HashExecutor; +import org.apache.geode.redis.internal.executor.hash.RedisHashCommands; +import org.apache.geode.redis.internal.executor.hash.RedisHashCommandsFunctionInvoker; import org.apache.geode.redis.internal.pubsub.PubSub; import org.apache.geode.redis.internal.statistics.RedisStats; @@ -79,6 +83,7 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { private final Runnable shutdownInvoker; private final RedisStats redisStats; private final EventLoopGroup subscriberGroup; + private final RedisHashCommandsFunctionInvoker hashCommands; private BigInteger scanCursor; private BigInteger sscanCursor; private BigInteger hscanCursor; @@ -126,6 +131,8 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { this.hscanCursor = new BigInteger("0"); redisStats.addClient(); + //TODO - this really should just be a cache wide field, not on the execution context + this.hashCommands = new RedisHashCommandsFunctionInvoker(getRegionProvider().getDataRegion()); // backgroundExecutor.submit(this::processCommandQueue); } @@ -133,7 +140,7 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { return channel.writeAndFlush(response.encode(byteBufAllocator), channel.newPromise()) .addListener((ChannelFutureListener) f -> { response.afterWrite(); - logResponse(response, channel.remoteAddress().toString(), f.cause()); + logResponse(response, channel.remoteAddress(), f.cause()); }); } @@ -343,7 +350,7 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { return response; } - private void logResponse(RedisResponse response, String extraMessage, Throwable cause) { + private void logResponse(RedisResponse response, Object extraMessage, Throwable cause) { if (logger.isDebugEnabled() && response != null) { ByteBuf buf = response.encode(new UnpooledByteBufAllocator(false)); if (cause == null) { @@ -469,4 +476,8 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { logger.info("Event loop interrupted", e); } } + + public RedisHashCommands getRedisHashCommands() { + return hashCommands; + } } diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisHashTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisHashTest.java index 446eea9..f4142d9 100644 --- a/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisHashTest.java +++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisHashTest.java @@ -56,11 +56,11 @@ public class RedisHashTest { } private RedisHash createRedisHash(String k1, String v1, String k2, String v2) { - ArrayList<ByteArrayWrapper> elements = new ArrayList<>(); - elements.add(createByteArrayWrapper(k1)); - elements.add(createByteArrayWrapper(v1)); - elements.add(createByteArrayWrapper(k2)); - elements.add(createByteArrayWrapper(v2)); + ArrayList<byte[]> elements = new ArrayList<>(); + elements.add(Coder.stringToBytes(k1)); + elements.add(Coder.stringToBytes(v1)); + elements.add(Coder.stringToBytes(k2)); + elements.add(Coder.stringToBytes(v2)); return new RedisHash(elements); } @@ -108,9 +108,9 @@ public class RedisHashTest { public void hset_stores_delta_that_is_stable() throws IOException { Region<RedisKey, RedisData> region = Mockito.mock(Region.class); RedisHash o1 = createRedisHash("k1", "v1", "k2", "v2"); - ByteArrayWrapper k3 = createByteArrayWrapper("k3"); - ByteArrayWrapper v3 = createByteArrayWrapper("v3"); - ArrayList<ByteArrayWrapper> adds = new ArrayList<>(); + byte[] k3 = Coder.stringToBytes("k3"); + byte[] v3 = Coder.stringToBytes("v3"); + ArrayList<byte[]> adds = new ArrayList<>(); adds.add(k3); adds.add(v3); o1.hset(region, null, adds, false); @@ -130,8 +130,8 @@ public class RedisHashTest { public void hdel_stores_delta_that_is_stable() throws IOException { Region<RedisKey, RedisData> region = mock(Region.class); RedisHash o1 = createRedisHash("k1", "v1", "k2", "v2"); - ByteArrayWrapper k1 = createByteArrayWrapper("k1"); - ArrayList<ByteArrayWrapper> removes = new ArrayList<>(); + byte[] k1 = Coder.stringToBytes("k1"); + ArrayList<byte[]> removes = new ArrayList<>(); removes.add(k1); o1.hdel(region, null, removes); assertThat(o1.hasDelta()).isTrue();
