Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

And one more fix to flaky tests#5467

Merged
bkchr merged 4 commits into
masterfrom
nv-fix-flaky
Mar 31, 2020
Merged

And one more fix to flaky tests#5467
bkchr merged 4 commits into
masterfrom
nv-fix-flaky

Conversation

@NikVolf
Copy link
Copy Markdown
Contributor

@NikVolf NikVolf commented Mar 31, 2020

(testing timers is hard, but it should nail it)

Closes #5144

@NikVolf NikVolf added the A0-please_review Pull request needs code review. label Mar 31, 2020
@NikVolf NikVolf added this to the 2.0 milestone Mar 31, 2020
@NikVolf NikVolf requested a review from tomusdrw as a code owner March 31, 2020 08:23
block_on(pool.maintain(block_event(1)));
assert_eq!(pool.status().ready, 1);
block_on(notifier.next());
block_on(notifier.next_blocking());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the difference between next and next_blocking?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

next_blocking first clears the queue from all previous timer events, then starts to wait next one (same as clear() + next() before)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm. If that is the case, what did change to the version before than? I don't see any difference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now it clears events after maintain, so if during maintain the timer is fired, it is not handled before maintain is finished 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ahh, so we just than wait one iteration more?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, one iteration that is guaranteed to be handled by timer handler

Comment thread Cargo.lock
[[package]]
name = "intervalier"
version = "0.3.0"
version = "0.3.1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be required in the Cargo.toml

block_on(pool.maintain(block_event(1)));
assert_eq!(pool.status().ready, 1);
block_on(notifier.next());
block_on(notifier.next_blocking());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm. If that is the case, what did change to the version before than? I don't see any difference.

// maintenance is in background
block_on(notifier.next());

let event = block_event_with_retracted(1, vec![retracted_hash]);
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.

We lost one test case it seems?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was duplicated code that didn't add anything

@bkchr bkchr added the B0-silent Changes should not be mentioned in any release notes label Mar 31, 2020
@bkchr bkchr merged commit a691281 into master Mar 31, 2020
@bkchr bkchr deleted the nv-fix-flaky branch March 31, 2020 10:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky transaction pool test

3 participants