dschneider-pivotal commented on a change in pull request #7172:
URL: https://github.com/apache/geode/pull/7172#discussion_r764350212
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -94,6 +101,16 @@ public RedisSet(int size) {
return diff;
}
+ private static int setOpStoreResult(RegionProvider regionProvider, RedisKey
destinationKey,
+ Set<byte[]> diff) {
+ if (diff.isEmpty()) {
+ regionProvider.getDataRegion().remove(destinationKey);
+ } else {
+ regionProvider.getLocalDataRegion().put(destinationKey, new
RedisSet(diff));
Review comment:
I'm pretty sure we don't need to be using a different region accessor
method for remove vs. put. Both of these are write ops so I believe
getLocalDataRegion has make no difference (it returns
PartitionRegionHelper.getLocalPrimaryData which only allows local reads ops but
the write ops can be local or remote).
Since getting getLocalPrimaryData when should probably prefer to use
getDataRegion() when doing write ops.
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -94,6 +101,16 @@ public RedisSet(int size) {
return diff;
}
+ private static int setOpStoreResult(RegionProvider regionProvider, RedisKey
destinationKey,
+ Set<byte[]> diff) {
+ if (diff.isEmpty()) {
+ regionProvider.getDataRegion().remove(destinationKey);
+ } else {
+ regionProvider.getLocalDataRegion().put(destinationKey, new
RedisSet(diff));
Review comment:
for the "region.put" case you should do a check to see if destinationKey
exists. If it doesn't do the put. But otherwise modify the existing RedisSet in
place and call storeChanges with a DeltaInfo. You will want to merge in the
latest develop to get the fix for GEODE-9772. The new DeltaType could be
SET_BYTE_ARRAYS and it would just be a List of byte[] like like
ADD_BYTE_ARRAYS. The difference is that when this DeltaInfo is applied you will
want to first call something on the RedisSet that clears it and calls
persistNoDelta and then just calls applyAddByteArrayDelta repeatedly. You will
also need to add a method on the near side (not the delta apply side) that does
this (given a Set<byte[]> does persistNoDelta() and "members = new
MemberSet(diff)". Even better would be to make sure calculateDIff always
returns a MemberSet instead of a Set and then this could just be "member =
diff". I think the only case when it is not a MemberSet is when it is empty in
which case you do a re
gion.remove so in this code you could just cast diff to MemberSet safely.
I can help with this delta work if you have any questions.
--
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]
