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

default flush timeout should be much lower, tunable to less than 1 second #757

Closed
bcantrill opened this issue May 29, 2023 · 0 comments · Fixed by #738
Closed

default flush timeout should be much lower, tunable to less than 1 second #757

bcantrill opened this issue May 29, 2023 · 0 comments · Fixed by #738
Assignees

Comments

@bcantrill
Copy link
Contributor

bcantrill commented May 29, 2023

When creating a Crucible upstairs, the flush_timeout is optionally specified in CrucibleOpts to be the number of seconds between flushes. Long flush times increase both space used by Crucible and time (as the number of outstanding jobs grows larger), but short flush times may result in excess CPU usage as fixed work is amortized over less time. When running measure-iops with the fixes described in #731 (implemented in #738), here is the effect of varying flush time:

2023-05-28-21-22-42_2109x1634

The default of 5 seconds is too long -- it leaves quite a bit of performance on the table. Moreover, it is clear that there is a case to be made for a sub-second flush timeout. To avoid changing the units or semantics of flush_timeout, we propose changing it from an unsigned integer to a float -- and to changing the default from 5.0 to 0.5 (that is, 500ms).

@bcantrill bcantrill self-assigned this May 29, 2023
bcantrill added a commit that referenced this issue May 30, 2023
In addition to fixing #731, this also fixes #757 -- and pulls in #733.

Co-authored-by: James MacMahon <james@oxide.computer>
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 a pull request may close this issue.

1 participant