jerrypeng commented on a change in pull request #10270:
URL: https://github.com/apache/pulsar/pull/10270#discussion_r616984700



##########
File path: 
pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java
##########
@@ -269,9 +270,11 @@ public void run() {
 
                 try {
                     processResult(currentRecord, result);
-                } catch (Exception e) {
+                    result.join();

Review comment:
       @david-streamlio I would recommend changing JavaInstance.handeMessage():
   
   
https://github.com/apache/pulsar/blob/master/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstance.java#L69
   
   I don't think it is appropriate to always return a 
CompletableFuture<JavaExecutionResult>.  We do not need to return a 
CompletableFuture for functions that are not implemented to use async.  I would 
recommend  modifying to existing class JavaExecutionResult to have options on 
whether to return a completable future result or just the result.




-- 
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:
[email protected]


Reply via email to