DonalEvans commented on a change in pull request #7507:
URL: https://github.com/apache/geode/pull/7507#discussion_r838781953
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##########
@@ -221,17 +221,18 @@ public long lpush(ExecutionHandlerContext context,
List<byte[]> elementsToAdd,
*/
public int lrem(int count, byte[] element, Region<RedisKey, RedisData>
region, RedisKey key) {
List<Integer> removedIndexes;
- byte version;
+ byte newVersion;
synchronized (this) {
removedIndexes = elementList.remove(element, count);
- version = incrementAndGetVersion();
- }
+ if (removedIndexes.isEmpty()) {
+ return 0;
+ }
- if (!removedIndexes.isEmpty()) {
- storeChanges(region, key,
- new RemoveElementsByIndex(version, removedIndexes));
+ newVersion = incrementAndGetVersion();
Review comment:
This change is not part of implementing RPUSHX and shouldn't really be
included here. Fixing multiple unrelated issues in one PR can make tracking
down the cause of bugs difficult, and cause trouble if things need to be
reverted. If there's a bug that's being fixed with this change, a separate PR
should be opened for it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
