Skip to content

[fix][broker]fix Repeated messages of shared dispatcher #18227

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Oct 28, 2022

Fixes #16795

Motivation

In Shard mode:

  1. When redelivering some message, will trigger the next read.
  2. When closing the lastest consumer or adding the first consumer, will trigger cursor.rewind which moves the read position to the earliest.

When the above two events are executed concurrently, the resulting redelivered message will be consumed twice.
E.g:

step close the last consumer & create 2 consumer redeliver
1 push message(3:1) to redeliver queue
2 trigger next read(readMoreEntries)
3 read entries(3:1) from the redeliver queue
4 clear the redeliver queue
5 set the read position of cursor to earliest(3:0)
6 trigger read event for the new consumer B get next consumer, the result is the new consumer A
7 read entries 3:0~3:10 send message(3:0) to the new consumer
8 send messages 3:0~3:10 to the new consumer B send message(3:0) to the new consumer A
9 bingo!

How to reproduce the problem

Run SimpleProducerConsumerTest.concurrentlyRedeliverAndCloseLastConsumer twice.

Modifications

Before step 7 of the flow redeliver, check that the messages are still in queue messagesToRedeliver.

Documentation

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

Matching PR in forked repository

PR in forked repository:

@codelipenghui
Copy link
Contributor

@poorbarcode Interesting. I think the expected behavior is no matter how many consumers are under a subscription. The subscription should only perform the entry read operation one by one.

Comment on lines +586 to +592
if (readType == ReadType.Replay) {
entries.removeIf(entry -> !redeliveryMessages.contains(entry.getLedgerId(), entry.getEntryId()));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I have tried to remove these lines to check the newly added test concurrentlyReceiveDeliveriedMessages

It always gets passed with invocationCount = 100

Copy link
Contributor Author

@poorbarcode poorbarcode Nov 1, 2022

Choose a reason for hiding this comment

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

I have modified the test concurrentlyRedeliverAndCloseLastConsumer to make that the probability of the problem occurring is greater than 50%.

@poorbarcode poorbarcode changed the title [fix][broker]fix Repeated messages of shared dispatcher [do not merge][fix][broker]fix Repeated messages of shared dispatcher Oct 28, 2022
@poorbarcode poorbarcode force-pushed the fix/shared_duplicate_message branch from 181efdf to bd153aa Compare November 1, 2022 18:14
@poorbarcode poorbarcode force-pushed the fix/shared_duplicate_message branch from bd153aa to 4302dc4 Compare November 1, 2022 18:36
@poorbarcode poorbarcode changed the title [do not merge][fix][broker]fix Repeated messages of shared dispatcher [fix][broker]fix Repeated messages of shared dispatcher Nov 1, 2022
@poorbarcode poorbarcode force-pushed the fix/shared_duplicate_message branch from 4302dc4 to d376c9a Compare November 10, 2022 13:38
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Dec 11, 2022
@poorbarcode poorbarcode modified the milestones: 3.0.0, 3.1.0 Apr 10, 2023
@Technoboy- Technoboy- modified the milestones: 3.1.0, 3.2.0 Jul 31, 2023
@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@coderzc coderzc removed this from the 3.3.0 milestone May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker doc-not-needed Your PR changes do not impact docs release/3.0.12 Stale type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky-test: SimpleProducerConsumerTest.testSharedSamePriorityConsumer
6 participants