[GitHub] [pulsar] eolivelli commented on a change in pull request #10653: [Transaction] Transaction admin api get coordinator internal stats.

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

eolivelli commented on a change in pull request #10653:
URL: https://github.com/apache/pulsar/pull/10653#discussion_r636075286



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TransactionsBase.java
##########
@@ -93,4 +105,109 @@ protected void internalGetCoordinatorStatus(AsyncResponse asyncResponse, boolean
                     "This Broker is not configured with transactionCoordinatorEnabled=true."));
         }
     }
+
+    protected void internalGetCoordinatorInternalStats(AsyncResponse asyncResponse, boolean authoritative,
+                                                       boolean metadata, int coordinatorId) {
+        try {
+            if (pulsar().getConfig().isTransactionCoordinatorEnabled()) {
+                TopicName topicName = TopicName.TRANSACTION_COORDINATOR_ASSIGN.getPartition(coordinatorId);
+                validateTopicOwnership(topicName, authoritative);
+                TransactionMetadataStore metadataStore = pulsar().getTransactionMetadataStoreService()
+                        .getStores().get(TransactionCoordinatorID.get(coordinatorId));
+                if (metadataStore instanceof MLTransactionMetadataStore) {
+                    asyncResponse.resume(getManageLedgerInternalStats(
+                            ((MLTransactionMetadataStore) metadataStore).getManagedLedger(),
+                            metadata, topicName.toString()).get());
+                } else {
+                    asyncResponse.resume(new RestException(METHOD_NOT_ALLOWED,
+                            "Broker don't use MLTransactionMetadataStore!"));
+                }
+            } else {
+                asyncResponse.resume(new RestException(SERVICE_UNAVAILABLE,
+                        "This Broker is not configured with transactionCoordinatorEnabled=true."));
+            }
+        } catch (Exception e) {
+            asyncResponse.resume(new RestException(e.getCause()));
+        }
+    }
+
+    private CompletableFuture<ManagedLedgerInternalStats> getManageLedgerInternalStats(ManagedLedger ledger,
+                                                                                       boolean includeLedgerMetadata,
+                                                                                       String topic) {
+        CompletableFuture<ManagedLedgerInternalStats> statFuture = new CompletableFuture<>();
+        ManagedLedgerInternalStats stats = new ManagedLedgerInternalStats();
+
+        ManagedLedgerImpl ml = (ManagedLedgerImpl) ledger;
+        stats.entriesAddedCounter = ml.getEntriesAddedCounter();

Review comment:
       what about moving this code to a method in ManagedLedgerImpl ?
   or at least some utility method that is out of this TransactionsBase class.
   
   this code belongs to ManagedLedgerImpl context, not the Transactions API

##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Transactions.java
##########
@@ -39,4 +40,14 @@
      */
     CompletableFuture<List<TransactionCoordinatorStatus>> getCoordinatorStatusList();
 
+    /**
+     * Get transaction coordinator internal stats.
+     *
+     * @param coordinatorId the coordinator id
+     * @param metadata is get ledger metadata
+     *
+     * @return the future internal stats of this coordinator
+     */
+    CompletableFuture<ManagedLedgerInternalStats> getCoordinatorInternalStats(int coordinatorId, boolean metadata);

Review comment:
       it is better to return a brand new type, dedicated to Coordinator, something like `CoordinatorInternalStats`.
   
   because they are separate domains, the ManagedLedger domain and the Transaction Coordinator.
   
   nothing prevents us to have an internal method that creates a CoordinatorInternalStats from a ManagedLedgerInternalStats, but that's an implementation detail




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