You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/25 08:07:50 UTC
[GitHub] [pulsar] HQebupt opened a new pull request, #16218: [improve][broker][PIP-149]make getBacklog method async
HQebupt opened a new pull request, #16218:
URL: https://github.com/apache/pulsar/pull/16218
Master Issue: #14013
### Motivation
See #14365
### Verifying this change
Make sure that the change passes the CI checks.
### 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: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [x] `doc-not-needed`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] HQebupt commented on pull request #16218: [improve][broker][PIP-149]make getBacklog method async
Posted by GitBox <gi...@apache.org>.
HQebupt commented on PR #16218:
URL: https://github.com/apache/pulsar/pull/16218#issuecomment-1167076419
/pulsarbot rerun-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 merged pull request #16218: [improve][broker][PIP-149]make getBacklog method async
Posted by GitBox <gi...@apache.org>.
Jason918 merged PR #16218:
URL: https://github.com/apache/pulsar/pull/16218
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16218: [improve][broker][PIP-149]make getBacklog method async
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #16218:
URL: https://github.com/apache/pulsar/pull/16218#discussion_r907917870
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -3040,44 +3040,46 @@ private Response generateResponseWithEntry(Entry entry) throws IOException {
return responseBuilder.entity(stream).build();
}
- protected PersistentOfflineTopicStats internalGetBacklog(boolean authoritative) {
+ protected CompletableFuture<PersistentOfflineTopicStats> internalGetBacklogAsync(boolean authoritative) {
+ CompletableFuture<Void> ret;
if (topicName.isGlobal()) {
- validateGlobalNamespaceOwnership(namespaceName);
+ ret = validateGlobalNamespaceOwnershipAsync(namespaceName);
+ } else {
+ ret = CompletableFuture.completedFuture(null);
}
// Validate that namespace exists, throw 404 if it doesn't exist
// note that we do not want to load the topic and hence skip authorization check
- try {
- namespaceResources().getPolicies(namespaceName);
- } catch (MetadataStoreException.NotFoundException e) {
- log.warn("[{}] Failed to get topic backlog {}: Namespace does not exist", clientAppId(), namespaceName);
- throw new RestException(Status.NOT_FOUND, "Namespace does not exist");
- } catch (Exception e) {
- log.error("[{}] Failed to get topic backlog {}", clientAppId(), namespaceName, e);
- throw new RestException(e);
- }
+ return ret.thenCompose(__ -> namespaceResources().getPoliciesAsync(namespaceName))
+ .thenCompose(__ -> {
+ PersistentOfflineTopicStats offlineTopicStats =
+ pulsar().getBrokerService().getOfflineTopicStat(topicName);
+ if (offlineTopicStats != null) {
+ // offline topic stat has a cost - so use cached value until TTL
+ long elapsedMs = System.currentTimeMillis() - offlineTopicStats.statGeneratedAt.getTime();
+ if (TimeUnit.MINUTES.convert(elapsedMs, TimeUnit.MILLISECONDS) < OFFLINE_TOPIC_STAT_TTL_MINS) {
+ return CompletableFuture.completedFuture(offlineTopicStats);
+ }
+ }
- PersistentOfflineTopicStats offlineTopicStats = null;
- try {
+ return pulsar().getBrokerService().getManagedLedgerConfig(topicName)
+ .thenCompose(config -> {
+ ManagedLedgerOfflineBacklog offlineTopicBacklog =
+ new ManagedLedgerOfflineBacklog(config.getDigestType(), config.getPassword(),
+ pulsar().getAdvertisedAddress(), false);
+ try {
+ PersistentOfflineTopicStats estimateOfflineTopicStats =
+ offlineTopicBacklog.estimateUnloadedTopicBacklog(
Review Comment:
I'm not sure, maybe we can make this method pure async.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #16218: [improve][broker][PIP-149]make getBacklog method async
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #16218:
URL: https://github.com/apache/pulsar/pull/16218#discussion_r907034682
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -3040,44 +3040,46 @@ private Response generateResponseWithEntry(Entry entry) throws IOException {
return responseBuilder.entity(stream).build();
}
- protected PersistentOfflineTopicStats internalGetBacklog(boolean authoritative) {
+ protected CompletableFuture<PersistentOfflineTopicStats> internalGetBacklogAsync(boolean authoritative) {
+ CompletableFuture<Void> ret;
if (topicName.isGlobal()) {
- validateGlobalNamespaceOwnership(namespaceName);
+ ret = validateGlobalNamespaceOwnershipAsync(namespaceName);
+ } else {
+ ret = CompletableFuture.completedFuture(null);
}
// Validate that namespace exists, throw 404 if it doesn't exist
// note that we do not want to load the topic and hence skip authorization check
- try {
- namespaceResources().getPolicies(namespaceName);
- } catch (MetadataStoreException.NotFoundException e) {
- log.warn("[{}] Failed to get topic backlog {}: Namespace does not exist", clientAppId(), namespaceName);
- throw new RestException(Status.NOT_FOUND, "Namespace does not exist");
- } catch (Exception e) {
- log.error("[{}] Failed to get topic backlog {}", clientAppId(), namespaceName, e);
Review Comment:
Do we have other places to print the logs? Please check we will not missed the logs after this PR.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org