8000 fix(subscriber): use saturating arithmetic for attribute updates by hawkw · Pull Request #234 · tokio-rs/console · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(subscriber): use saturating arithmetic for attribute updates #234

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

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

hawkw
Copy link
Member
@hawkw hawkw commented Dec 20, 2021

Currently, an issue exists where a subtraction attribute update for a
resource or async op attribute may overflow, if a previous add update
was dropped due to event buffer capacity. This may result in the
aggregator task panicking. See
#180 (comment)
for details.

This branch changes all resource updates to use saturating arithmetic.
In practice, I think it's much more likely for a subtract to overflow
due to missed adds than it is for an add to overflow due to missed subs,
but I made the addition saturating as well, just in case.

We should probably also increase the default event buffer capacity,
since it seems like lots of people are hitting problems with this...

Currently, an issue exists where a subtraction attribute update for a
resource or async op attribute may overflow, if a previous add update
was dropped due to event buffer capacity. This may result in the
aggregator task panicking. See
#180 (comment)
for details.

This branch changes all resource updates to use saturating arithmetic.
In practice, I think it's _much_ more likely for a subtract to overflow
due to missed adds than it is for an add to overflow due to missed subs,
but I made the addition saturating as well, just in case.

We should probably also increase the default event buffer capacity,
since it seems like lots of people are hitting problems with this...
@hawkw hawkw added S-bug Severity: bug C-subscriber Crate: console-subscriber. labels Dec 20, 2021
@hawkw hawkw self-assigned this Dec 20, 2021
hawkw added a commit that referenced this pull request Dec 20, 2021
It seems like a number of people using the console with real life
applications are running into issues related to the console's event
buffer being at capacity (e.g. #230, #234, etc). This commit bumps up
the default capacity a bit as a quick fix.

In the future, I think we should probably do things to avoid reaching
the event buffer capacity as often --- I think we can change the
aggregator/layer design a bit to avoid creating as many events. But, for
now, this should hopefully give people a better default configuration.
Copy link
Collaborator
@zaharidichev zaharidichev left a comment

Choose a reason for hiding this comment

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

Good catch!

@hawkw hawkw merged commit fe82e17 into main Dec 20, 2021
@hawkw hawkw deleted the eliza/saturating-sub branch December 20, 2021 19:39
hawkw added a commit that referenced this pull request Dec 21, 2021
It seems like a number of people using the console with real life
applications are running into issues related to the console's event
buffer being at capacity (e.g. #230, #234, etc). This commit bumps up
the default capacity a bit as a quick fix.

In the future, I think we should probably do things to avoid reaching
the event buffer capacity as often --- I think we can change the
aggregator/layer design a bit to avoid creating as many events. But, for
now, this should hopefully give people a better default configuration.
hawkw added a commit that referenced this pull request Jan 18, 2022
<a name="0.1.1"></a>
## 0.1.1 (2022-01-18)

#### Bug Fixes

*  only send *new* tasks/resources/etc over the event channel (#238) ([fdc77e2](fdc77e2))
*  increased default event buffer capacity (#235) ([0cf0aee](0cf0aee))
*  use saturating arithmetic for attribute updates (#234) ([fe82e17](fe82e17))

#### Changes

*  moved ID rewriting from `console-subscriber` to the client (#244) ([095b1ef](095b1ef))
## 0.1.0 (2021-12-16)

- Initial release! &#x1f389;
hawkw added a commit that referenced this pull request Jan 18, 2022
<a name="0.1.1"></a>
## 0.1.1 (2022-01-18)

#### Bug Fixes

*  only send *new* tasks/resources/etc over the event channel (#238) ([fdc77e2](fdc77e2))
*  increased default event buffer capacity (#235) ([0cf0aee](0cf0aee))
*  use saturating arithmetic for attribute updates (#234) ([fe82e17](fe82e17))

#### Changes

*  moved ID rewriting from `console-subscriber` to the client (#244) ([095b1ef](095b1ef))
## 0.1.0 (2021-12-16)

- Initial release! &#x1f389;
hawkw added a commit that referenced this pull request Jan 18, 2022
<a name="0.1.1"></a>
## 0.1.1 (2022-01-18)

#### Bug Fixes

*  only send *new* tasks/resources/etc over the event channel (#238) ([fdc77e2](fdc77e2))
*  increased default event buffer capacity (#235) ([0cf0aee](0cf0aee))
*  use saturating arithmetic for attribute updates (#234) ([fe82e17](fe82e17))

#### Changes

*  moved ID rewriting from `console-subscriber` to the client (#244) ([095b1ef](095b1ef))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-subscriber Crate: console-subscriber. S-bug Severity: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0