dschneider-pivotal commented on a change in pull request #6814:
URL: https://github.com/apache/geode/pull/6814#discussion_r703750211
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
##########
@@ -97,6 +97,7 @@ public PubSubImpl(Subscriptions subscriptions) {
registerPublishFunction();
Review comment:
The pubSub function is not serialized in any way. This is from the
comment on Function:
* Even though this interface extends Serializable, functions will only be
serialized if they are
* not registered. For best performance it is recommended that you implement
{@link #getId()} to
* return a non-null identifier and register your function using {@link
* FunctionService#registerFunction(Function)}
For clarity I also think this code might be easier to understand if the
function was a top level class instead of an anonymous inner class. I have
found it difficult the first couple times I tried to find the function to do
so. It would also give a place to add a comment about this function not being
serialized. But I think that belongs in another pr. I created another jira
issue the pubsub function execution (GEODE-9575) perhaps you could add a
comment on it about cleaning up the function code itself.
--
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]
