dschneider-pivotal commented on a change in pull request #6704:
URL: https://github.com/apache/geode/pull/6704#discussion_r674993888
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
##########
@@ -58,23 +62,25 @@ public int getSubscriptionCount() {
@Override
public long publish(RegionProvider regionProvider, byte[] channel, byte[]
message) {
- Set<DistributedMember> membersWithDataRegion =
regionProvider.getRegionMembers();
- @SuppressWarnings("unchecked")
- ResultCollector<String[], List<Long>> subscriberCountCollector =
FunctionService
- .onMembers(membersWithDataRegion)
- .setArguments(new Object[] {channel, message})
- .execute(REDIS_PUB_SUB_FUNCTION_ID);
+ executor.submit(() -> internalPublish(regionProvider, channel, message));
- List<Long> subscriberCounts = null;
+ return subscriptions.findSubscriptions(channel).size();
Review comment:
consider calling findSubscriptions(channel).size() first and saving its
value in "long result".
Then only call executor.submit if result > 0.
Then return result.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSub.java
##########
@@ -31,10 +31,8 @@
/**
* Publish a message on a channel
*
- *
* @param channel to publish to
* @param message to publish
- * @return the number of messages published
*/
long publish(RegionProvider regionProvider, byte[] channel, byte[] message);
Review comment:
since publish still returns a long it seems like you should still have a
comment describing what is returned. How about "return an estimate of the
number of messages published by this call". It might also be worth enhancing
this comment to say that this call may return before the actual publish has
been done. Something about it being async.
--
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]
