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 2022/06/27 20:33:05 UTC

[GitHub] [pulsar] poorbarcode opened a new pull request, #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

poorbarcode opened a new pull request, #16248:
URL: https://github.com/apache/pulsar/pull/16248

   ### Motivation
   E.g. `client.lookup(by producer)`, `topic.unload`, `consumer.subscribe`  executed at the same time:
   
   | Time | `client.lookup(by producer)` | `topic.unload` | `consumer.subscribe` |
   | ----------- | ----------- | ----------- | ----------- | 
   | 1 |  |  | `ServerCnx.consumers.putIfAbsent(consumerId, consumerFuture)` |
   | 2 | get existing persistent topic |  |  |
   | 3 | create a new persistent subscription |  |  |
   | 4 | create a new pending ack handle |  |  |
   | 5 | repaly async |  |  |
   | 6 |  |  | waiting for pending ack log repaly finish |
   | 6 |  | topic.close |  |
   | 7 |  | async close subscription |  |
   | 8 |  | change pending ack handle state  --> `close` |  |
   | 9 | change pending ack handle state `init` --> `ready` |  |  |
   | 10 | change state failure |  |  |
   | 11 |  |  | subscribe timeout |
   | 12 |  |  | retry subscribe |
   | 13 |  |  | get exists `consumerFuture` in `ServerCnx.consumers` |
   | 14 |  |  | waiting for pending ack log repaly finish |
   | 15 |  |  | subscribe timeout |
   | 16 |  |  | ......    (loop step12 ~ step15) |
   
   Step 6/14: `PersistentSubscription.addConsumer` will waiting for pending ack replay done.
   
   https://github.com/apache/pulsar/blob/ebcc47ee7ceb43f680640ad72e51a06d9856458d/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java#L205-L206
   
   Step 10: When failure to modify the pending-ack-handle-state will not terminate the `pendingAckHandleFuture`
   
   https://github.com/apache/pulsar/blob/ebcc47ee7ceb43f680640ad72e51a06d9856458d/pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/MLPendingAckReplyCallBack.java#L46-L61
   
   After step 11: Cmd-Subscribe will not succeed even after 100 retries.
   
   ### Modifications
   
   When failure to modify the pending-ack-handle-state, make `pendingAckHandleFuture` exceptionally complete.
   
   
   ### Documentation
   
   Check the box below or label this PR directly.
   
   Need to update docs? 
   
   - [ ] `doc-required` 
   (Your PR needs to update docs, and you will update later)
     
   - [x] `doc-not-needed` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)


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


[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#discussion_r907808461


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java:
##########
@@ -131,7 +132,12 @@ public void recoverComplete() {
                                 maxReadPosition = (PositionImpl) topic.getManagedLedger().getLastConfirmedEntry();
                             }
                             if (!changeToReadyState()) {
-                                log.error("[{}]Transaction buffer recover fail", topic.getName());
+                                log.error("[{}]Transaction buffer recover fail, current state: {}",
+                                        topic.getName(), getState());
+                                transactionBufferFuture.completeExceptionally
+                                        (new TransactionBufferException.TransactionBufferRecoverException(
+                                                "Transaction buffer recover fail, change state to Ready failure."

Review Comment:
   ```suggestion
                                                   "Transaction buffer recover failed to change the status to Ready,"
   ```



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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1169449376

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1167858982

   @congbobo184 @liangyepianzhou  Please take a look, 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.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1170896715

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] congbobo184 merged pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
congbobo184 merged PR #16248:
URL: https://github.com/apache/pulsar/pull/16248


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


[GitHub] [pulsar] congbobo184 commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1170652690

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1170838988

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1169538233

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1170760932

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#discussion_r910614806


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentTopicTest.java:
##########
@@ -795,7 +795,7 @@ public void testChangeSubscriptionType() throws Exception {
     public void testAddRemoveConsumer() throws Exception {
         PersistentTopic topic = new PersistentTopic(successTopicName, ledgerMock, brokerService);
         PersistentSubscription sub = new PersistentSubscription(topic, "sub-1", cursorMock, false);
-
+        topic.getSubscriptions().put("sub-1", sub);

Review Comment:
   I have removed it



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


[GitHub] [pulsar] RobertIndie commented on a diff in pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
RobertIndie commented on code in PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#discussion_r907956787


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/MLPendingAckReplyCallBack.java:
##########
@@ -53,8 +54,10 @@ public void replayComplete() {
                 log.info("Topic name : [{}], SubName : [{}] pending ack handle cache request success!",
                         pendingAckHandle.getTopicName(), pendingAckHandle.getSubName());
             } else {
-                log.error("Topic name : [{}], SubName : [{}] pending ack state reply fail!",
-                        pendingAckHandle.getTopicName(), pendingAckHandle.getSubName());
+                log.error("Topic name : [{}], SubName : [{}] pending ack state reply fail! current state: {}",
+                        pendingAckHandle.getTopicName(), pendingAckHandle.getSubName(), pendingAckHandle.state);
+                replayFailed(new TransactionPendingAckException.TransactionPendingAckStoreReplayException("Change"
+                        + " PendingAckHandle state to Ready failure, current state is : " + pendingAckHandle.state));

Review Comment:
   ```suggestion
                   replayFailed(new TransactionPendingAckException.TransactionPendingAckStoreReplayException("Failed"
                           + "to change PendingAckHandle state to Ready, current state is : " + pendingAckHandle.state));
   ```



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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1168354342

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1169076719

   Hi @congbobo184 
   
   > Nice catch! please add some test for this pr
   
   I have already appended the unit test, please take a look 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


[GitHub] [pulsar] poorbarcode commented on pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#issuecomment-1170708014

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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] congbobo184 commented on a diff in pull request #16248: [fix] [transaction] Cmd-Subscribe and Cmd-Producer will not succeed even after 100 retries

Posted by GitBox <gi...@apache.org>.
congbobo184 commented on code in PR #16248:
URL: https://github.com/apache/pulsar/pull/16248#discussion_r909138090


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PersistentTopicTest.java:
##########
@@ -795,7 +795,7 @@ public void testChangeSubscriptionType() throws Exception {
     public void testAddRemoveConsumer() throws Exception {
         PersistentTopic topic = new PersistentTopic(successTopicName, ledgerMock, brokerService);
         PersistentSubscription sub = new PersistentSubscription(topic, "sub-1", cursorMock, false);
-
+        topic.getSubscriptions().put("sub-1", sub);

Review Comment:
   why add this line



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