Skip to content

fix: bug in delayed messaging which prevented some messages from getting delivered#5489

Closed
jerrypeng wants to merge 2 commits into
apache:masterfrom
jerrypeng:fix_delayed_messaging
Closed

fix: bug in delayed messaging which prevented some messages from getting delivered#5489
jerrypeng wants to merge 2 commits into
apache:masterfrom
jerrypeng:fix_delayed_messaging

Conversation

@jerrypeng
Copy link
Copy Markdown
Contributor

Motivation

There is a bug in the delayed messaging delivery code which can skip delivery of messages.
If there is already a pending read i.e. havePendingReplayRead = true, messages are not delivered but never added back into the delayed message tracker, thus causing these messages to never to be delivered.

Modifications

When there is a pending read in progress, i.e. havePendingReplayRead=true, add the messages back into the delayed message tracker so they will get send the next time around.

@jerrypeng jerrypeng added the type/bug The PR fixed a bug or issue reported a bug label Oct 29, 2019
@jerrypeng jerrypeng added this to the 2.4.2 milestone Oct 29, 2019
@jerrypeng jerrypeng self-assigned this Oct 29, 2019
Copy link
Copy Markdown
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Looks good to me

@Test
public void test() throws Exception {

String topic = "testNegativeAcks-" + System.nanoTime();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems this test not related to negative acks.

@merlimat
Copy link
Copy Markdown
Contributor

I think there's a much simpler fix to this issue. The root cause is we're getting out the messages out of the tracker but not doing anything with that. Will post separate PR.

@jerrypeng
Copy link
Copy Markdown
Contributor Author

withdrawing in favor of #5499

@jerrypeng jerrypeng closed this Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants