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 2021/05/09 08:00:08 UTC

[GitHub] [pulsar] linlinnn opened a new pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

linlinnn opened a new pull request #10520:
URL: https://github.com/apache/pulsar/pull/10520


   **Motivation**
   handle startMessageRollbackDurationSec when get durable subscription


-- 
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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   /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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   /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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   > Can you please add a test case?
   @eolivelli any suggestion about this, `MockManagedLedger` do nothing.
   


-- 
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] eolivelli merged pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #10520:
URL: https://github.com/apache/pulsar/pull/10520


   


-- 
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] linlinnn removed a comment on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

Posted by GitBox <gi...@apache.org>.
linlinnn removed a comment on pull request #10520:
URL: https://github.com/apache/pulsar/pull/10520#issuecomment-837644341






-- 
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] linlinnn edited a comment on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

Posted by GitBox <gi...@apache.org>.
linlinnn edited a comment on pull request #10520:
URL: https://github.com/apache/pulsar/pull/10520#issuecomment-836318900


   > Can you please add a test case?
   
   @eolivelli any suggestion about this, `MockManagedLedger` do nothing.
   


-- 
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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   /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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   @eolivelli I have added unit test, please review again, thanks


-- 
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] gaoran10 commented on a change in pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -893,24 +898,29 @@ public void openCursorFailed(ManagedLedgerException exception, Object ctx) {
             }
 
             if (startMessageRollbackDurationSec > 0) {
-                long timestamp = System.currentTimeMillis()
-                        - TimeUnit.SECONDS.toMillis(startMessageRollbackDurationSec);
-                final Subscription finalSubscription = subscription;
-                subscription.resetCursor(timestamp).handle((s, ex) -> {
-                    if (ex != null) {
-                        log.warn("[{}] Failed to reset cursor {} position at timestamp {}", topic, subscriptionName,
-                                startMessageRollbackDurationSec);
-                    }
-                    subscriptionFuture.complete(finalSubscription);
-                    return null;
-                });
+                resetSubscriptionCursor(subscription, subscriptionFuture, startMessageRollbackDurationSec);
                 return subscriptionFuture;
             } else {
                 return CompletableFuture.completedFuture(subscription);
             }
         }
     }
 
+    private void resetSubscriptionCursor(Subscription subscription, CompletableFuture<Subscription> subscriptionFuture,
+                                         long startMessageRollbackDurationSec) {
+        long timestamp = System.currentTimeMillis()
+                - TimeUnit.SECONDS.toMillis(startMessageRollbackDurationSec);
+        final Subscription finalSubscription = subscription;
+        subscription.resetCursor(timestamp).handle((s, ex) -> {
+            if (ex != null) {

Review comment:
       If the reset cursor operation is failed, the subscription still could be generated successfully, it seems that the configuration startMessageRollbackDurationSec is invalid, this has no effect on the users, right?
   
   Maybe we could print a simple error message in the warning log.




-- 
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] linlinnn removed a comment on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

Posted by GitBox <gi...@apache.org>.
linlinnn removed a comment on pull request #10520:
URL: https://github.com/apache/pulsar/pull/10520#issuecomment-836318900


   > Can you please add a test case?
   
   @eolivelli any suggestion about this, `MockManagedLedger` do nothing.
   


-- 
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] linlinnn commented on pull request #10520: get durable subscription without handling startMessageRollbackDurationSec

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


   /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