[GitHub] [pulsar] congbobo184 opened a new pull request #10648: [Transaction] Transaction admin api transaction in pendingack stats

[GitBox <gi...@apache.org>]

congbobo184 opened a new pull request #10648:
URL: https://github.com/apache/pulsar/pull/10648


   ## Motivation
   Transaction add admin api `getTransactionInPendingAckStatss`
   
   ## implement
   ```
   public class TransactionInPendingAckStats {
   
       /** The topic of this transaction ack. */
       public String topic;
   
       /** The subscription name of this transaction sub. */
       public String subName;
   
       /** The state of this transaction pending ack. */
       public String state;
   
       /** The position of this transaction cumulative ack. */
       public String cumulativeAckPosition;
   
       /** The positions of this transaction individual ack. */
       public List<String> individualAckPosition;
   }
   ```
   This is transaction buffer metrics.
   ### Verifying this change
   Add the tests for it
   
   Does this pull request potentially affect one of the following parts:
   If yes was chosen, please highlight the changes
   
   Dependencies (does it add or upgrade a dependency): (no)
   The public API: (no)
   The schema: (no)
   The default values of configurations: (no)
   The wire protocol: (no)
   The rest endpoints: (no)
   The admin cli options: (yes)
   Anything that affects deployment: (no)
   
   


-- 
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] codelipenghui merged pull request #10648: [Transaction] Transaction admin api transaction in pendingack stats

[GitBox <gi...@apache.org>]

codelipenghui merged pull request #10648:
URL: https://github.com/apache/pulsar/pull/10648


   


-- 
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] codelipenghui commented on a change in pull request #10648: [Transaction] Transaction admin api transaction in pendingack stats

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org