Skip to content

Add more unit tests for the priority mempool.#8961

Merged
creachadair merged 2 commits intov0.35.xfrom
mjf/pm-test
Jul 7, 2022
Merged

Add more unit tests for the priority mempool.#8961
creachadair merged 2 commits intov0.35.xfrom
mjf/pm-test

Conversation

@creachadair
Copy link

@creachadair creachadair commented Jul 7, 2022

  • Add a test for time-based (TTL) expiration.
  • Add a test for eviction priority.

Add more unit tests for the priority mempool.
34c3891 
- Add a test for time-based (TTL) expiration.
- Add a test for eviction priority.
williambanfield
williambanfield approved these changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@williambanfield williambanfield left a comment

Choose a reason for hiding this comment

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

These look great. The thing I'm most worried about test-wise is the size x priority eviction logic. Are you planning to add that in a follow up PR?

@creachadair
Copy link
Author

creachadair commented Jul 7, 2022 •
edited
Loading

These look great. The thing I'm most worried about test-wise is the size x priority eviction logic. Are you planning to add that in a follow up PR?

Yeah. There are a couple tests for the reaping logic, but I think there are a couple more cases we should add. I could do them in this PR if you'd prefer, I mostly wanted to keep the chunks smaller for review.

Edit to add: Or is that not what you meant?

@williambanfield
Copy link
Contributor

williambanfield commented Jul 7, 2022

Yeah. There are a couple tests for the reaping logic, but I think there are a couple more cases we should add. I could do them in this PR if you'd prefer, I mostly wanted to keep the chunks smaller for review.

Either is fine, just want to make sure that it's on your radar.

@creachadair
Copy link
Author

creachadair commented Jul 7, 2022

These look great. The thing I'm most worried about test-wise is the size x priority eviction logic. Are you planning to add that in a follow up PR?

Per our offline discussion—I've added a few more cases to hit what I hope you had in mind. Please let me know if there are other cases you think are worth testing.

@creachadair creachadair merged commit bc49f66 into v0.35.x Jul 7, 2022
@creachadair creachadair deleted the mjf/pm-test branch July 7, 2022 21:56
creachadair pushed a commit that referenced this pull request Jul 8, 2022
Backport priority mempool fixes from v0.35.x to v0.34.x.
d17d253 
This is a manual backport of the changes from these commits:

- bc49f66 Add more unit tests for the priority mempool. (#8961)
- 9b02094 Fix unbounded heap growth in the priority mempool. (#8944)

Imports and type signatures have been updated to match the v0.34 usage.
@creachadair creachadair mentioned this pull request Jul 8, 2022
Merged
creachadair pushed a commit that referenced this pull request Jul 8, 2022
Backport priority mempool fixes from v0.35.x to v0.34.x. (#8962)
493dd69 
This is a manual backport of the changes from these commits:

- bc49f66 Add more unit tests for the priority mempool. (#8961)
- 9b02094 Fix unbounded heap growth in the priority mempool. (#8944)

Imports and type signatures have been updated to match the v0.34 usage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebuchman ebuchman Awaiting requested review from ebuchman

@cmwaters cmwaters Awaiting requested review from cmwaters

@tychoish tychoish Awaiting requested review from tychoish

1 more reviewer

@williambanfield williambanfield williambanfield approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@creachadair @williambanfield