[GitHub] [pulsar] Jason918 commented on a diff in pull request #16236: [improve][java-client] Replace ScheduledExecutor to improve performance of message consumption

[GitBox <gi...@apache.org>]

Jason918 commented on code in PR #16236:
URL: https://github.com/apache/pulsar/pull/16236#discussion_r907276620


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -1378,10 +1379,11 @@ private ByteBuf processMessageChunk(ByteBuf compressedPayload, MessageMetadata m
 
         // Lazy task scheduling to expire incomplete chunk message
         if (!expireChunkMessageTaskScheduled && expireTimeOfIncompleteChunkedMessageMillis > 0) {
-            internalPinnedExecutor
-                    .scheduleAtFixedRate(catchingAndLoggingThrowables(this::removeExpireIncompleteChunkedMessages),
-                            expireTimeOfIncompleteChunkedMessageMillis, expireTimeOfIncompleteChunkedMessageMillis,
-                            TimeUnit.MILLISECONDS);
+            ((ScheduledExecutorService) client.getScheduledExecutorProvider().getExecutor()).scheduleAtFixedRate(

Review Comment:
   
   `client.getScheduledExecutorProvider().getExecutor()` this gets a thread in roundrobin. But currently, some of the race conditions are avoided by using this single thread executor `internalPinnedExecutor`. Have you checked if this new scheduled executor thread would introduce some race conditions? 
   
   Or can we just wrap these tasks into `internalPinnedExecutor` again?



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org