Skip to content
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

[fix] [broker] Fix metrics pulsar_topic_load_failed_count is 0 when load non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed #22580

Merged
merged 1 commit into from Apr 29, 2024

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Apr 25, 2024

Fixes: #20495

Motivation

  1. The test BrokerServiceTest.testBrokerStatsTopicLoadFailed always fails, so [fix][test] Disable invalid test BrokerServiceTest#testBrokerStatsTopicLoadFailed #20494 disabled it.
  2. There is a case that pulsar_topic_load_failed_count does not increase when load non-persistent topic fails[1].

[1]: https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1239-L1258

CompletableFuture<Optional<Topic>> topicFuture = new CompletableFuture<>();
topicFuture.exceptionally(t -> {
    pulsarStats.recordTopicLoadFailed();
    return null;
});
try {
    nonPersistentTopic = newTopic(topic, null, this, NonPersistentTopic.class);
} catch (Throwable e) {
    log.warn("Failed to create topic {}", topic, e);
    // Highlight: It is a new future, so the event `topicFuture.exceptionally` will not receive the error callback anymore.
    return FutureUtil.failedFuture(e);
}

Modifications

  1. fix the test.
  2. fix the issue

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 3.3.0 milestone Apr 25, 2024
@poorbarcode poorbarcode self-assigned this Apr 25, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 25, 2024
@poorbarcode poorbarcode changed the title [fix] [broker] [fix] [broker] Fix metrics pulsar_topic_load_failed_count is 0 when load non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed [fix] [broker] Fix metrics pulsar_topic_load_failed_count is 0 when load non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed Apr 28, 2024
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

…oad non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed
@poorbarcode
Copy link
Contributor Author

Rebase master

@lhotari lhotari merged commit 340d60d into apache:master Apr 29, 2024
51 of 52 checks passed
poorbarcode added a commit that referenced this pull request May 2, 2024
…oad non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed (#22580)

(cherry picked from commit 340d60d)
poorbarcode added a commit that referenced this pull request May 2, 2024
…oad non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed (#22580)

(cherry picked from commit 340d60d)
poorbarcode added a commit that referenced this pull request May 2, 2024
…oad non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed (#22580)

(cherry picked from commit 340d60d)
poorbarcode added a commit that referenced this pull request May 2, 2024
…oad non-persistent topic fails and fix the flaky test testBrokerStatsTopicLoadFailed (#22580)

(cherry picked from commit 340d60d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite BrokerServiceTest#testBrokerStatsTopicLoadFailed
4 participants