You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/07/31 12:07:12 UTC

[GitHub] [pulsar] Tiscs opened a new pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Tiscs opened a new pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710


   Fix #7707
   Fix message consume ordering issue of function with EFFECTIVELY_ONCE processing guarantees.
   
   
   ### Motivation
   
   Since v2.5.2, function with EFFECTIVELY_ONCE processing guarantees consume message out of ordering when fail, because process a CompletableFuture result by async.
   
   ### Modifications
   
   - Get result from future synchronized when processing guarantees of java instance equals `EFFECTIVELY_ONCE`.
   


----------------------------------------------------------------
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



[GitHub] [pulsar] jiazhai commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-690002238


   This pr seems brings too much un-related changes by the merge. Would you please follow [this guide](http://pulsar.apache.org/en/contributing/#clone-the-repository-locally) to provide a new PR?


----------------------------------------------------------------
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



[GitHub] [pulsar] Tiscs commented on a change in pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs commented on a change in pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#discussion_r483355016



##########
File path: pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java
##########
@@ -415,28 +415,45 @@ private void setupStateTable() throws Exception {
         }
     }
 
-    private void processResult(Record srcRecord,
-                               CompletableFuture<JavaExecutionResult> result) throws Exception {
-        result.whenComplete((result1, throwable) -> {
-            if (throwable != null || result1.getUserException() != null) {
-                Throwable t = throwable != null ? throwable : result1.getUserException();
-                log.warn("Encountered exception when processing message {}",
-                        srcRecord, t);
-                stats.incrUserExceptions(t);
-                srcRecord.fail();
+    private void processResult(Record srcRecord, JavaExecutionResult result, Throwable throwable) throws Exception {
+        throwable = throwable == null ? result.getUserException() : throwable;
+        if (result.getUserException() != null) {

Review comment:
       Should replace `result.getUserException()` to local variable `throwable` 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



[GitHub] [pulsar] sijie commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-686862251


   @Tiscs Can you rebase the pull request to latest master 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.

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



[GitHub] [pulsar] Tiscs commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-689587476


   > @Tiscs Thanks for the contribution, it seems there are still some conflicts with the master. Would you please help rebase this PR again?
   
   Rebased again, and fixed exception detection in `processResult`, but some checks were failed.


----------------------------------------------------------------
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



[GitHub] [pulsar] jiazhai commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-689530293


   @Tiscs Thanks for the contribution, it seems there are still some conflicts with the master. Would you please help rebase this PR 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.

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



[GitHub] [pulsar] Tiscs commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-690026669


   Closed by new PR #8026.


----------------------------------------------------------------
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



[GitHub] [pulsar] jiazhai commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-667788278


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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



[GitHub] [pulsar] Tiscs closed pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs closed pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710


   


----------------------------------------------------------------
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



[GitHub] [pulsar] Tiscs commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-678977667


   Rebased from `origin/master`.


----------------------------------------------------------------
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



[GitHub] [pulsar] Tiscs commented on pull request #7710: [pulsar-functions] Fix message consume ordering issue of function

Posted by GitBox <gi...@apache.org>.
Tiscs commented on pull request #7710:
URL: https://github.com/apache/pulsar/pull/7710#issuecomment-686866665


   > @Tiscs Can you rebase the pull request to latest master again?
   
   @sijie Done.


----------------------------------------------------------------
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