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][test] Fix flaky DelayedDeliveryTest.testEnableTopicDelayedDelivery #23893

Merged

Conversation

pdolif
Copy link
Contributor

@pdolif pdolif commented Jan 24, 2025

Fixes #23882

Motivation

The test class uses a delayedDeliveryTickTimeMillis of 1024.
In the flaky test, a message is produced that should be delivered after 2 seconds. Then, consumer.receive() is called with a timeout of 1 second and it is asserted that no message is received. Because of the tick time of 1024ms, the message may already be delivered after 2s-1024ms=976ms. In this case, the test fails because a message is consumed within 1 second.

Modifications

Configure the message to be delivered after 3 seconds (instead of 2).
This ensures that the message is delivered at the earliest after 3s-1024ms=1976ms. The consumer.receive() with 1 second timeout should not receive the message anymore.

After that, there is another consumer.receive() whose timeout was increased from 3 to 4 seconds.

Verifying this change

  • Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: pdolif#7

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 24, 2025
@merlimat merlimat merged commit 9079262 into apache:master Jan 28, 2025
53 of 54 checks passed
lhotari pushed a commit that referenced this pull request Jan 28, 2025
@lhotari
Copy link
Member

lhotari commented Jan 29, 2025

Noticed this issue in another test method in the same class after cherry-picking to branch-3.0 and running in branch-3.0: #23906

lhotari pushed a commit that referenced this pull request Jan 29, 2025
lhotari pushed a commit that referenced this pull request Jan 29, 2025
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 31, 2025
…ery (apache#23893)

(cherry picked from commit 9079262)
(cherry picked from commit f9d3dbf)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 3, 2025
…ery (apache#23893)

(cherry picked from commit 9079262)
(cherry picked from commit f9d3dbf)
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.

Flaky-test: DelayedDeliveryTest.testEnableTopicDelayedDelivery
3 participants