-
Notifications
You must be signed in to change notification settings - Fork 142
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: flaky CurrentEventsByTagTest #783
Conversation
CI check passed, close and reopn PR to trigger CI check run again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check failed again, the same error occurred.
@@ -191,6 +203,8 @@ abstract class CurrentEventsByTagTest(config: String) extends QueryTestSpec(conf | |||
|
|||
// start the query before the last batch completes | |||
journalOps.withCurrentEventsByTag()(tag, NoOffset) { tp => | |||
// when query begin running, unlock the barrier. | |||
latch.countDown() | |||
// The stream must complete within the given amount of time | |||
// This make take a while in case the journal sequence actor detects gaps | |||
val allEvents = tp.toStrict(atMost = 20.seconds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flaky/unstable assert on here, It depends on the execution speed.
|
||
// wait for acknowledgement of the first batch only | ||
batch1.futureValue | ||
// Sanity check, all events in the first batch must be in the journal | ||
journalOps.countJournal.futureValue should be >= 600L | ||
|
||
// Try to persist a large batch of events per actor. Some of these may be returned, but not all! | ||
val batch2 = sendMessagesWithTag(tag, 5000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this reordering works, CI check passed twice, trigger check again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for trying this out, sorry for letting it sit here for so long.
LGTM.
9b64693
to
9c8b5a5
Compare
References #782
Delay persistence to the start of the query