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

[Flaky Test]: Fix flaky test TestMaxPendingChunkMessages() #1003

Merged
merged 2 commits into from
May 11, 2023

Conversation

Gleiphir2769
Copy link
Contributor

Fixes #983

Motivation

Old TestMaxPendingChunkMessages() uses the concurrent message publish to make the consumer discard unavailable chunk. And it's flaky.

So the sendSingeChunk() is introduced to manual create scenarios where old chunks should be discarded.

Modifications

  • Fix TestMaxPendingChunkMessages()

Verifying this change

  • Make sure that the change passes the CI checks.

@Gleiphir2769 Gleiphir2769 marked this pull request as draft April 8, 2023 14:50
@Gleiphir2769 Gleiphir2769 changed the title [fix]: Fix flaky test TestMaxPendingChunkMessages() [Flaky Test]: Fix flaky test TestMaxPendingChunkMessages() Apr 8, 2023
@Gleiphir2769 Gleiphir2769 marked this pull request as ready for review April 9, 2023 13:16
Comment on lines 565 to 573
producerImpl.internalSingleSend(
mm,
msg.Payload,
&sendRequest{
callback: func(id MessageID, producerMessage *ProducerMessage, err error) {},
msg: msg,
},
uint32(internal.MaxMessageSize),
)
Copy link
Member

Choose a reason for hiding this comment

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

We need to flush the producer after sending the message. Or handle the callback of the sendRequest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to flush the producer after sending the message

OK, I will fix it.

@shibd shibd requested a review from RobertIndie May 6, 2023 02:55
@shibd
Copy link
Member

shibd commented May 6, 2023

@RobertIndie Can you retake a look?

@Gleiphir2769
Copy link
Contributor Author

Ping @RobertIndie.

@RobertIndie RobertIndie merged commit 3367cc0 into apache:master May 11, 2023
@RobertIndie RobertIndie added this to the v0.11.0 milestone Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky Test: TestMaxPendingChunkMessages
3 participants