[GitHub] [pulsar] rdhabalia commented on a change in pull request #3786: [pulsar-function] fix deadlock in pulsar-source

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

rdhabalia commented on a change in pull request #3786: [pulsar-function] fix deadlock in pulsar-source
URL: https://github.com/apache/pulsar/pull/3786#discussion_r263967432
 
 

 ##########
 File path: pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/source/PulsarSource.java
 ##########
 @@ -100,7 +98,15 @@ public void open(Map<String, Object> config, SourceContext sourceContext) throws
             }
 
             return cb.subscribeAsync();
-        }).collect(Collectors.toList()).stream().map(CompletableFuture::join).collect(Collectors.toList());
+        }).collect(Collectors.toList()).stream().map(consumerFuture -> {
+            try {
+                return consumerFuture.get(consumerOpTimeoutMs, TimeUnit.MILLISECONDS);
 
 Review comment:
   I don't think it's a hack but hack was made in #3749, where we were interrupting and killing thread without knowing reason. And After checking thread-dump I have found out that function-thread was not completing because it was blocked on this path. 
   However, you are correct that we also have to find out root-cause of why consumer-future not was completed and therefore we have to add the timeout. I haven't found anything into the log yet. But I found out that function-worker code-path has many blocking calls and same time it deals with many locks so, it's good practice to avoid forever-blocking call to give a chance to break a deadlock. Therefore, we have added a timeout here.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services