Github user galen-pivotal commented on a diff in the pull request:
https://github.com/apache/geode/pull/404#discussion_r105053042
--- Diff:
geode-core/src/main/java/org/apache/geode/redis/internal/executor/set/SMoveExecutor.java
---
@@ -45,30 +48,44 @@ public void executeCommand(Command command,
ExecutionHandlerContext context) {
checkDataType(source, RedisDataType.REDIS_SET, context);
checkDataType(destination, RedisDataType.REDIS_SET, context);
- @SuppressWarnings("unchecked")
- Region<ByteArrayWrapper, Boolean> sourceRegion =
- (Region<ByteArrayWrapper, Boolean>)
context.getRegionProvider().getRegion(source);
- if (sourceRegion == null) {
+ Region<ByteArrayWrapper, Set<ByteArrayWrapper>> region =
getRegion(context);
+
+ Set<ByteArrayWrapper> sourceSet = region.get(source);
+
+ if (sourceSet == null) {
command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(),
NOT_MOVED));
return;
}
- Object oldVal = sourceRegion.get(mem);
- sourceRegion.remove(mem);
+ sourceSet = new HashSet<ByteArrayWrapper>(sourceSet); // copy to
support transactions;
--- End diff --
Copying helps with concurrent modifications on the same machine, but we can
still lose set adds 'cause race conditions (A reads, B reads, A adds key "foo"
to a copy, B adds key "bar" to a copy, A puts, B puts, the key added by A is
lost). <s>I think the `ExecutionHandlerContext` provides transaction
support...</s> txn support in `ExecutionHandlerContext` looks like it's only
for MULTI/EXEC.
I think we can do transactions in Geode by locking a particular key, or two
keys if they're on the same server. However, if they aren't then we'll just
have to say "whoops!" and return an error (Redis cluster does the same thing).
Maybe just mark it as a FIXME for now, and carry on?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
