[jira] [Commented] (GEODE-8059) Use a single function to route redis commands to primary

[ASF GitHub Bot (Jira)]

    [ 
https://issues.apache.org/jira/browse/GEODE-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17097215#comment-17097215
 ] 

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_r418446095



##########
File path: 
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/GeodeRedisSetWithFunctions.java
##########
@@ -40,30 +47,18 @@ public GeodeRedisSetWithFunctions(ByteArrayWrapper key,
   }
 
   public static void registerFunctions() {
-    FunctionService.registerFunction(new SaddFunction());
-    FunctionService.registerFunction(new SremFunction());
-    FunctionService.registerFunction(new SmembersFunction());
-    FunctionService.registerFunction(new SdelFunction());
+    FunctionService.registerFunction(new CommandFunction());
   }
 
   @Override
   public long sadd(ArrayList<ByteArrayWrapper> membersToAdd) {
-    ResultCollector<ArrayList<ByteArrayWrapper>, List<Long>> results = 
FunctionService
-        .onRegion(region)
-        .withFilter(Collections.singleton(key))
-        .setArguments(membersToAdd)
-        .execute(SaddFunction.ID);
-
+    ResultCollector<Object[], List<Long>> results = executeFunction(SADD, 
membersToAdd);
     return results.getResult().get(0);

Review comment:
       srem is different but I think we could make it the same. And then you 
have the generic challenge of getting executeFunction to return a generic value 
that the caller can specify. This seems doable and I like having as much of the 
common code in executeFunction as possible.




----------------------------------------------------------------
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)