codelipenghui commented on a change in pull request #10648:
URL: https://github.com/apache/pulsar/pull/10648#discussion_r637544532



##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/PendingAckHandleImpl.java
##########
@@ -699,6 +700,21 @@ public void completeHandleFuture() {
         
this.pendingAckHandleCompletableFuture.complete(PendingAckHandleImpl.this);
     }
 
+    @Override
+    public TransactionInPendingAckStats getTransactionInPendingAckStats(TxnID 
txnID) {
+        TransactionInPendingAckStats transactionInPendingAckStats = new 
TransactionInPendingAckStats();
+        if (cumulativeAckOfTransaction != null && 
cumulativeAckOfTransaction.getLeft().equals(txnID)) {
+            transactionInPendingAckStats.cumulativeAckPosition = 
this.cumulativeAckOfTransaction.getRight().toString();

Review comment:
       If using the position.toString(), seems it's not able to expose the 
batch index information.

##########
File path: 
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTransactions.java
##########
@@ -41,8 +42,30 @@ void run() throws Exception {
         }
     }
 
+    @Parameters(commandDescription = "Get transaction in pending ack stats")
+    private class GetTransactionInPendingAckStats extends CliCommand {
+        @Parameter(names = {"-c", "--coordinator-id"}, description = "the 
coordinator id", required = true)
+        private int coordinatorId;
+
+        @Parameter(names = {"-id", "--sequencce-id"}, description = "the 
sequenceId", required = true)
+        private int sequenceId;

Review comment:
       `sequenceId` is confusing here because this is also a  concept in the 
producer side, it's better to use `--transactionId` here? Such as 
--transactionId (0,123456)

##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v3/Transactions.java
##########
@@ -53,4 +54,30 @@ public void getCoordinatorStatus(@Suspended final 
AsyncResponse asyncResponse,
                                      @QueryParam("coordinatorId") Integer 
coordinatorId) {
         internalGetCoordinatorStatus(asyncResponse, authoritative, 
coordinatorId);
     }
+
+    @GET
+    @Path("/transactionInPendingAckStats")
+    @ApiOperation(value = "Get transaction state in pending ack.")
+    @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have 
admin permission"),
+            @ApiResponse(code = 404, message = "Tenant or cluster or namespace 
or topic doesn't exist"),
+            @ApiResponse(code = 503, message = "This Broker is not configured "
+                    + "with transactionCoordinatorEnabled=true."),
+            @ApiResponse(code = 500, message = "Topic don't owner by this 
broker!"),

Review comment:
       should be 307 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:
[email protected]


Reply via email to