jdeppe-pivotal commented on a change in pull request #7403:
URL: https://github.com/apache/geode/pull/7403#discussion_r819568306
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##########
@@ -82,28 +92,93 @@ public long lpush(List<byte[]> elementsToAdd,
Region<RedisKey, RedisData> region
*/
public byte[] lpop(Region<RedisKey, RedisData> region, RedisKey key) {
byte[] popped = elementRemove(0);
- RemoveElementsByIndex removed = new RemoveElementsByIndex();
+ RemoveElementsByIndex removed = new RemoveElementsByIndex(version);
removed.add(0);
storeChanges(region, key, removed);
return popped;
}
/**
- * @return the number of elements in the list
+ * @param elementsToAdd elements to add to this set; NOTE this list may be
modified by this call
+ * @param region the region this instance is stored in
+ * @param key the name of the set to add to
+ * @return the number of elements actually added
*/
- public int llen() {
+ public long lpush(List<byte[]> elementsToAdd, Region<RedisKey, RedisData>
region, RedisKey key) {
+ elementsPush(elementsToAdd);
+ storeChanges(region, key, new AddByteArraysVersioned(version,
elementsToAdd));
return elementList.size();
}
+ public byte[] ltrim(long start, long end, Region<RedisKey, RedisData> region,
+ RedisKey key) {
+ int length = elementList.size();
+ int preservedVersion;
+ int boundedStart = getBoundedStartIndex(start, length);
+ int boundedEnd = getBoundedEndIndex(end, length);
+ List<Integer> removed = new ArrayList<>();
+
+ synchronized (this) {
+ if (boundedStart > boundedEnd || boundedStart == length) {
+ // Remove everything
+ for (int i = length - 1; i >= 0; i--) {
+ removed.add(i);
+ }
+ } else {
+ // Remove any elements after boundedEnd
+ for (int i = length - 1; i > boundedEnd; i--) {
+ removed.add(i);
+ }
+
+ for (int i = boundedStart - 1; i >= 0; i--) {
+ removed.add(i);
+ }
+ }
+
+ if (removed.size() > 0) {
+ elementsRemove(removed);
+ }
+ preservedVersion = version;
+ }
+ System.out.println(
+ "DEBUG ltrim, preserved version:" + preservedVersion + " (version:" +
version + ")");
+ storeChanges(region, key, new RemoveElementsByIndex(preservedVersion,
removed));
+ return null;
+ }
+
+ private int getBoundedStartIndex(long index, int size) {
+ if (index >= 0L) {
+ return (int) Math.min(index, size);
+ } else {
+ return (int) Math.max(index + size, 0);
+ }
+ }
+
+ private int getBoundedEndIndex(long index, int size) {
+ if (index >= 0L) {
+ return (int) Math.min(index, size);
+ } else {
+ return (int) Math.max(index + size, -1);
+ }
+ }
+
@Override
- public void applyAddByteArrayDelta(byte[] bytes) {
- elementPush(bytes);
+ public void applyAddByteArrayVersionedDelta(int version, byte[] bytes) {
+ System.out.println("DEBUG abav: local version:" + this.version + "
incoming:" + version);
+ if (version != this.version) {
+ elementPush(bytes);
+ }
+ this.version = version;
}
@Override
- public void applyRemoveElementsByIndex(List<Integer> indexes) {
- for (int index : indexes) {
- elementRemove(index);
+ public void applyRemoveElementsByIndex(int version, List<Integer> indexes) {
+ System.out.println("DEBUG arebi: local version:" + this.version + "
incoming:" + version);
+ synchronized (this) {
+ if (version != this.version) {
+ elementsRemove(indexes);
+ }
+ this.version = version;
Review comment:
There shouldn't be any need to synchronize here since this is a delta
application and there will be locking at the Geode level to protect the entry.
I think this should just be:
```
if (version > this.version) {
elementsRemove(indexes);
this.version = version;
}
```
You want to update the version to match the change being applied. Similarly
in `applyAddByteArrayVersionedDelta`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
