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 test NonPersistentTopicTest.testMsgDropStat #20387

Merged
merged 1 commit into from
May 24, 2023

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 24, 2023

Fixes #20386

Motivation

See #20386

Modifications

Use Awaitility in test. Consume and acknowledge messages with messageListener.

Documentation

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

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the effective change is the retry part, while the receive and ack part won't be regressions. +1 to merge.

@tisonkun
Copy link
Member

Merging...

Thank you!

@tisonkun tisonkun merged commit 903425b into apache:master May 24, 2023
@tisonkun tisonkun added this to the 3.1.0 milestone May 24, 2023
lhotari added a commit to lhotari/pulsar that referenced this pull request May 25, 2023
…llow up

- the previous attempt apache#20387 to fix the flakiness wasn't effective
- improve the fix
michaeljmarshall pushed a commit that referenced this pull request May 25, 2023
…llow up (#20401)

Fixes #20386

### Motivation

- the previous attempt #20387 to fix the flakiness wasn't effective

### Modifications

- improve the fix and rely on the fact that entryId is -1 when the message is dropped in the broker
  code: https://github.com/apache/pulsar/blob/091ee2504ffbe6ec98e354b76e7f4c045e1914aa/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L1699-L1711

### Documentation

<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->

- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test type/flaky-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky-test: NonPersistentTopicTest.testMsgDropStat
3 participants