lovelle commented on a change in pull request #3337: Bug fixes/Improvement for
notify pending receive method
URL: https://github.com/apache/pulsar/pull/3337#discussion_r247335066
##########
File path:
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -910,27 +910,38 @@ void messageReceived(MessageIdData messageId, int
redeliveryCount, ByteBuf heade
* @param message
*/
void notifyPendingReceivedCallback(final Message<T> message, Exception
exception) {
- if (!pendingReceives.isEmpty()) {
- // fetch receivedCallback from queue
- CompletableFuture<Message<T>> receivedFuture =
pendingReceives.poll();
- if (exception == null) {
- checkNotNull(message, "received message can't be null");
- if (receivedFuture != null) {
- if (conf.getReceiverQueueSize() == 0) {
- // return message to receivedCallback
- receivedFuture.complete(message);
- } else {
- // increase permits for available message-queue
- Message<T> interceptMsg = beforeConsume(message);
- messageProcessed(interceptMsg);
- // return message to receivedCallback
- listenerExecutor.execute(() ->
receivedFuture.complete(interceptMsg));
- }
- }
- } else {
- listenerExecutor.execute(() ->
receivedFuture.completeExceptionally(exception));
- }
+ if (pendingReceives.isEmpty()) {
+ return;
}
+
+ // fetch receivedCallback from queue
+ final CompletableFuture<Message<T>> receivedFuture =
pendingReceives.poll();
+ if (receivedFuture == null) {
+ return;
+ }
+
+ if (exception != null) {
+ listenerExecutor.execute(() ->
receivedFuture.completeExceptionally(exception));
+ return;
+ }
+
+ if (message == null) {
+ NullPointerException np = new NullPointerException("received
message can't be null");
+ listenerExecutor.execute(() ->
receivedFuture.completeExceptionally(np));
+ throw np;
+ }
+
+ final Runnable completeReceivedCallback = () ->
receivedFuture.complete(beforeConsume(message));
+ if (conf.getReceiverQueueSize() == 0) {
+ // return message to receivedCallback
+ listenerExecutor.execute(completeReceivedCallback);
+ return;
+ }
+
+ // increase permits for available message-queue
+ messageProcessed(message);
Review comment:
Totally agree with you, at a first attempt I thought it would be better but
if user interceptor takes too long we would be scheduling too much tasks on
executor.
As you said, it would be better to leave the interceptor call on current
thread rather than potentially another thread. I'm changing this the way it was
before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
