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

reactor: change shares for default IO class from 1 to 200 #1706

Closed

Conversation

michoecho
Copy link
Contributor

The number of shares is supposed to be set in the 1-1000 range, and user-defined IO classes have shares on the order of hundreds in practice.

Therefore, 1 share is not a sane default, because the default class can very easily be starved by accident.
We have seen in practice for example in scylladb/scylladb#13753.

The number of shares is supposed to be set in the 1-1000 range,
and user-defined IO classes have shares on the order of hundreds
in practice.

Therefore, 1 share is not a sane default, because the default class
can very easily be starved by accident.
We have seen in practice for example in scylladb/scylladb#13753.
@mykaul
Copy link

mykaul commented Jun 19, 2023

There's no 'backport' label here, but I assume our intention is to pick this up for stable versions.

@michoecho
Copy link
Contributor Author

@avikivity I'm not sure if we want this change in seastar master (this could be a backwards-incompatible change for some users), or only in Scylla's stable forks of Seastar. What's your judgement?

@michoecho
Copy link
Contributor Author

There's no 'backport' label here, but I assume our intention is to pick this up for stable versions.

Yes.

@dorlaor
Copy link
Contributor

dorlaor commented Jun 19, 2023 via email

@michoecho
Copy link
Contributor Author

michoecho commented Jun 19, 2023

Is it a regression or shares==1 has always been there?

Since f259b31 (so: since forever).

@michoecho
Copy link
Contributor Author

Ping.

@michoecho
Copy link
Contributor Author

@avikivity @xemul Bump. Do we want this change in seastar master (this could be a backwards-incompatible change for some users), or only in scylla-seastar?

@avikivity
Copy link
Member

This code is obsolete in API level 7. So let's go for minimum disruption and only apply the change to ScyllaDB.

@michoecho
Copy link
Contributor Author

This code is obsolete in API level 7. So let's go for minimum disruption and only apply the change to ScyllaDB.

Then please create branch-5.3 in scylla-seastar, so we can backport seastar fixes to 5.3.

@avikivity
Copy link
Member

Done.

@michoecho
Copy link
Contributor Author

Transferred to scylladb/scylla-seastar#3.

@michoecho michoecho closed this Jun 21, 2023
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.

None yet

4 participants