cckellogg commented on a change in pull request #10853:
URL: https://github.com/apache/pulsar/pull/10853#discussion_r648504885
##########
File path:
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java
##########
@@ -1578,6 +1578,41 @@ public void failed(Throwable throwable) {
}
}
+ @Override
+ public Long getBacklogSizeByMessageId(String topic, String messageId,
boolean checkLedgerExists)
Review comment:
There is a `MessageId` object we should use that instead of a string
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
##########
@@ -1458,6 +1458,33 @@ public PersistentOfflineTopicStats getBacklog(
return internalGetBacklog(authoritative);
}
+ @GET
+
@Path("/{tenant}/{namespace}/{topic}/messageId/{messageId}/checkLedgerExists/{checkLedgerExists}/backlogSize")
Review comment:
This API is confusing. Why is a message ID required? If it's not
provides can it default to the latest?
This `checkLedgerExists/{checkLedgerExists}` should not be part of the path
in my opinion. If you need this `checkLedgerExists` it should be passed as a
query parameter.
I still don't understand why `checkLedgerExists` is needed. Can't the
response return an error if it does not exist?
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/PersistentTopics.java
##########
@@ -662,6 +662,32 @@ public PersistentOfflineTopicStats
getBacklog(@PathParam("property") String prop
return internalGetBacklog(authoritative);
}
+ @GET
+
@Path("/{property}/{cluster}/{namespace}/{topic}/messageId/{messageId}/checkLedgerExists/{checkLedgerExists}/backlogSize")
Review comment:
Do we need to support V1 for new apis like this?
--
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]
