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.

Sign up for GitHub

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

Jump to bottom

Capacity change + overflow mode #1

Merged
merged 4 commits into from
May 17, 2021
Merged

Capacity change + overflow mode #1

merged 4 commits into from
May 17, 2021

Conversation

zeenix
Copy link
Member

@zeenix zeenix commented May 13, 2021 •
edited
Loading

This PR adds:

  • Add a notion of message priority (used by the other two API). In the end, this will just complicate things and I don't see people wanting to delete newer messages on channel shrink
  • Ability to set capacity after the channel has been created. If the new capacity is smaller than the current length, messages are dropped to correct the situation. The messages are dropped starting from oldest to newest.
  • Overflow mode setting: When enabled, broadcasting a message to a full channel always succeeds, by removal of the oldest message from the channel. The removed message is handed over the Sender::{try_broadcast, broadcast} caller.

@zeenix zeenix requested a review from yoshuawuyts May 13, 2021 16:46
@zeenix zeenix marked this pull request as draft May 14, 2021 14:12
@zeenix
Copy link
Member Author

zeenix commented May 14, 2021

Converting it into a draft as this is broken for awaiting receivers. I'm working on it..

@zeenix zeenix mentioned this pull request May 15, 2021
Closed
@zeenix
Copy link
Member Author

zeenix commented May 15, 2021

Gah, the calculations/algorithm required here turned out to be quite a pickle. I have pushed my WIP work here and with the last commit the existing and the newly added test succeed now but testing it against zbus, reveals it's still broken as messages just get lost. So I could really use some help here or at least a second pair of eyes.

@yoshuawuyts @elmarco @tv42 ^

@zeenix zeenix force-pushed the overflow-mode branch 2 times, most recently from 9b34586 to 6459670 Compare May 16, 2021 11:15
@zeenix
Copy link
Member Author

zeenix commented May 16, 2021 •
edited
Loading

The WIP work I just pushed seems to work fine. There is still at least one issue (if latest messages are dropped on capacity change, receivers are wrongly adjusted) and need to optimize the code a bit but hopefully the hard part is done and I can fix this today.

@zeenix
Add ability to set channel capacity
63c040a 
If new capacity is less then channel length, oldest messages are
dropped.
@zeenix
tests: Bring back sleep() to ensure awaiting
5537302
@zeenix
Add overflow mode setting
f4f37c5 
When enabled, broadcasting a message to a full channel always succeeds,
by removal of the oldest message from the channel.

This is an API break.
@zeenix
Bump minor version due to API break
7c1a358
@zeenix zeenix marked this pull request as ready for review May 16, 2021 19:17
@zeenix
Copy link
Member Author

zeenix commented May 16, 2021

I think I got it working, based on all the testing I can give it. If anyone else can have a look and tell me if this can be improved, please do let me know.

@zeenix
Copy link
Member Author

zeenix commented May 17, 2021

All my testing has been green so far, so I'm merging this already. We can always fix things later if needed.

@zeenix zeenix merged commit 1e3119a into master May 17, 2021
@zeenix zeenix deleted the overflow-mode branch May 17, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yoshuawuyts yoshuawuyts Awaiting requested review from yoshuawuyts

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@zeenix