-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Transactionn] Transaction admin api getTransactionBufferStatus and getPendingAckStatus #10650
[Transactionn] Transaction admin api getTransactionBufferStatus and getPendingAckStatus #10650
Conversation
…_admin_api # Conflicts: # pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java
…nto congbobo184_transaction_admin_api_component_in_topic_status
…t_in_topic_status # Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TransactionsBase.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v3/Transactions.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/TransactionBuffer.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/InMemTransactionBuffer.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TransactionBufferDisable.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/PendingAckHandle.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/PendingAckHandleDisabled.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/PendingAckHandleImpl.java # pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java # pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Transactions.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TransactionsImpl.java # pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java # pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTransactions.java
@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 = 307, message = "Topic don't owner by this broker!"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about
"Topic is not owned by this broker"
@ApiResponse(code = 503, message = "This Broker is not configured " | ||
+ "with transactionCoordinatorEnabled=true."), | ||
@ApiResponse(code = 307, message = "Topic don't owner by this broker!"), | ||
@ApiResponse(code = 501, message = "Topic is not a persistent topic!"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Topic is not a persistent topic"
501 is kind of "internal server error", probably we should return something like "Not allowed" or "Bad request"
public void getTransactionInBufferStats(@Suspended final AsyncResponse asyncResponse, | ||
@QueryParam("authoritative") | ||
@DefaultValue("false") boolean authoritative, | ||
@QueryParam("mostSigBits") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it a little bit awkward to to pass separately mostSigBits and leastSigBits in a REST API ?
can we using a string representation ?
do we have other examples in the REST API about splitting the TxID into the two parts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client don't know the txnId is consists of coordinatorId and sequenceId. it only know a TxnID consists of mostSigBits and leastSigBits. It's better to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eolivelli Currently all the txn Id in the admin API expressed as mostSigBits
and leastSigBits
. The client also can get the mostSigBits
and leastSigBits
of a transactionId when using the transactions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check all the name which use **Status
, we are using **Stats
before.
*/ | ||
package org.apache.pulsar.common.policies.data; | ||
|
||
public class TransactionBufferStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class TransactionBufferStatus { | |
public class TransactionBufferStats { |
*/ | ||
package org.apache.pulsar.common.policies.data; | ||
|
||
public class TransactionPendingAckStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class TransactionPendingAckStatus { | |
public class TransactionPendingAckStats { |
@@ -42,6 +42,31 @@ void run() throws Exception { | |||
} | |||
} | |||
|
|||
@Parameters(commandDescription = "Get transaction buffer status") | |||
private class GetTransactionBufferStatus extends CliCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private class GetTransactionBufferStatus extends CliCommand { | |
private class GetTransactionBufferStats extends CliCommand { |
} | ||
|
||
@Parameters(commandDescription = "Get transaction pending ack status") | ||
private class GetPendingAckStatus extends CliCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private class GetPendingAckStatus extends CliCommand { | |
private class GetPendingAckStats extends CliCommand { |
@@ -84,6 +109,8 @@ void run() throws Exception { | |||
public CmdTransactions(Supplier<PulsarAdmin> admin) { | |||
super("transactions", admin); | |||
jcommander.addCommand("coordinator-status", new GetCoordinatorStatus()); | |||
jcommander.addCommand("transaction-buffer-status", new GetTransactionBufferStatus()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jcommander.addCommand("transaction-buffer-status", new GetTransactionBufferStatus()); | |
jcommander.addCommand("transaction-buffer-stats", new GetTransactionBufferStats()); |
@@ -84,6 +109,8 @@ void run() throws Exception { | |||
public CmdTransactions(Supplier<PulsarAdmin> admin) { | |||
super("transactions", admin); | |||
jcommander.addCommand("coordinator-status", new GetCoordinatorStatus()); | |||
jcommander.addCommand("transaction-buffer-status", new GetTransactionBufferStatus()); | |||
jcommander.addCommand("pending-ack-status", new GetPendingAckStatus()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jcommander.addCommand("pending-ack-status", new GetPendingAckStatus()); | |
jcommander.addCommand("pending-ack-stats", new GetPendingAckStats()); |
…t_in_topic_status # Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/TransactionsBase.java # pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v3/Transactions.java # pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/v3/AdminApiTransactionTest.java # pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Transactions.java # pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TransactionsImpl.java # pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java
@Anonymitaet please review doc thanks. |
* Get transaction pending ack stats. | ||
* | ||
* @param topic the topic of this transaction pending ack stats | ||
* @param subName the topic of this transaction pending ack stats |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @param subName the topic of this transaction pending ack stats | |
* @param subName the subName of this transaction pending ack stats |
is subName
accurate? or subscription name
?
@congbobo184 Could you please update the title of the PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect
…etPendingAckStatus (apache#10650)
…etPendingAckStatus (apache#10650)
Motivation
Transaction add admin api
getTransactionBufferStatus
Transaction add admin api
getPendingAckStatus
implement
This is transaction component status.
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)