[ https://issues.apache.org/jira/browse/GEODE-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17097222#comment-17097222 ]
ASF GitHub Bot commented on GEODE-8059: --------------------------------------- dschneider-pivotal commented on a change in pull request #5035: URL: https://github.com/apache/geode/pull/5035#discussion_r418447712 ########## File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/DeltaSet.java ########## @@ -30,10 +30,37 @@ import org.apache.geode.Delta; import org.apache.geode.InvalidDeltaException; import org.apache.geode.cache.Region; +import org.apache.geode.cache.execute.ResultSender; import org.apache.geode.redis.internal.ByteArrayWrapper; public class DeltaSet implements Delta, DataSerializable { + public static void sadd(ResultSender<Long> resultSender, + Region<ByteArrayWrapper, DeltaSet> localRegion, ByteArrayWrapper key, + ArrayList<ByteArrayWrapper> membersToAdd) { + resultSender.lastResult(sadd(localRegion, key, membersToAdd)); Review comment: ResultSender is just an interface so someone else could implement it. But I think at this point we don't want anyone except the Function interacting with DeltaSet. We have some work to do (the other set ops that directly read the region) but once they all use the function I'm okay with the only public methods on DeltaSet being ones that encourage the methods to only be used by Functions. But I would also be okay with DeltaSet not even having these ResultSender methods and instead the DeltaSet methods return a value and then we have some code in the Function that adds it to the ResultSender. At some point I'd like the static methods on DeltaSet to go away. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Use a single function to route redis commands to primary > -------------------------------------------------------- > > Key: GEODE-8059 > URL: https://issues.apache.org/jira/browse/GEODE-8059 > Project: Geode > Issue Type: Improvement > Components: redis > Reporter: Darrel Schneider > Assignee: Darrel Schneider > Priority: Major > > The redis implementation now uses a function to route the execution of a > redis command to the server that has the primary for a particular key. > Currently a function exists for each command (sadd, srem, smembers, del). > Each time we want to do this for another redis command it is a bunch of work > if we add another function. > Instead we could have a single function that takes an enum describing what > command it is being asked to execute. -- This message was sent by Atlassian Jira (v8.3.4#803005)