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

admission: replace soft slots mechanism with elastic cpu tokens #88032

Closed
3 tasks done
irfansharif opened this issue Sep 16, 2022 · 2 comments · Fixed by #95590
Closed
3 tasks done

admission: replace soft slots mechanism with elastic cpu tokens #88032

irfansharif opened this issue Sep 16, 2022 · 2 comments · Fixed by #95590
Assignees
Labels
A-admission-control C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-storage Storage Team

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Sep 16, 2022

Is your feature request related to a problem? Please describe.

We introduced a notion of soft slots in admission control (#78519) for additional threads for Pebble compaction compression. We envisioned granting these "squishy" slots to background activities and permit work only under periods of low load. In working through #86638 (as part of #75066), we observed experimentally that the moderate-slots count was not sensitive enough to scheduling latency, and consequently latency observed by foreground traffic. Elastic CPU tokens, the kind now being used for backups, offer an alternative to soft slots. This issue tracks:

  • Replacing the use of soft slots in additional pebble compaction compression threads
  • Running some sanity check experiments as part of this (perhaps those we ran in admission: introduce soft slots for speculative concurrency #78519)
  • Removing the code around soft/moderate load slots (it's better to minimize the number of mechanisms in the admission package)

Additional context

Some internal discussion here. Also #87883 could help highlight the (lack of) sensitivity to scheduling latency, perhaps worth checking again. Note that the elastic CPU limiter is disabled for 22.2, so if we're looking to do this in 22.2 (likely too late in the release), we'd have to rework the feature gates slightly to only block the integration with ExportRequests.

+cc @sumeerbhola, @andrewbaptist, @bananabrick.

Jira issue: CRDB-19649

Epic CRDB-20293

@irfansharif irfansharif added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-admission-control labels Sep 16, 2022
@bananabrick bananabrick self-assigned this Sep 16, 2022
@nicktrav nicktrav added the T-storage Storage Team label Sep 26, 2022
@blathers-crl blathers-crl bot added the A-storage Relating to our storage engine (Pebble) on-disk storage. label Sep 26, 2022
@exalate-issue-sync exalate-issue-sync bot removed the A-storage Relating to our storage engine (Pebble) on-disk storage. label Oct 6, 2022
@nicktrav
Copy link
Collaborator

@bananabrick - in #90791, it looks like Cockroach got the vendor bump with your recent changes, but the work granter code in pkg/server/config.go was commented out. I assume this means the work granting mechanism is effectively disabled on master right now.

What's left to do here?

@irfansharif
Copy link
Contributor Author

What's left to do here?

Just deleting some dead AC code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-admission-control C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-storage Storage Team
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants