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.

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(accumulator): Allow zero duration granularity #9

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Feb 29, 2024

A zero duration currently panics in chrono: chronotope/chrono#1474

I need support for zero duration granularity to always immediately flush after recording a value, this is very useful for (integration)-tests.

Also changes the flush check from > to >= to always flush with a zero duration granularity.

Copy link
Collaborator

@fpacifici fpacifici left a comment

Choose a reason for hiding this comment

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

Could you provide some context on the issues you are getting in the integration tests?
You should always be able to force a flush of the accumulator in your test no matter on the granularity

@Dav1dde
Copy link
Member Author

Dav1dde commented Mar 4, 2024

Could you provide some context on the issues you are getting in the integration tests?
You should always be able to force a flush of the accumulator in your test no matter on the granularity

The aggregation interval is configurable in Relay via config (as seconds), which ends up getting passed to rust-usage-accountant. In an integration test I want to just change Relay config while at the same time not introduce additional config just for the integration test (something a user should never configure). It is very convenient if I can set the interval to 0 and it just does the right thing, 1) no aggregation 2) immediate flush. Overall this means less code and easier parameterizable tests.

@Dav1dde Dav1dde merged commit 1000027 into main Mar 4, 2024
2 checks passed
@Dav1dde Dav1dde deleted the dav1d/fix/panic-zero-duration branch April 5, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants