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

Make Resource Group operations executed in SingleThreadExecutor #18669

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rmarduga
Copy link
Contributor

@rmarduga rmarduga commented Nov 11, 2022

Problem description
Presently, all operations on Resource Group tree-like data structure are protected by the root group monitor. That makes all operation are blockable. Essentially, these operations are the bottleneck for the Coordinator. It limits the number of queries that could be admitted by Coordinator. Additionally, those operations are invoked from many different threads that could be scheduled on different cores. That hurts the performance of those operations due to the necessity of running costly cache coherence operations to invalidate local caches on different hardware cores. That limits the bandwidth on operations over Resource Groups.

Proposed solution
move all the operations on the resource group tree to a single thread (using Single Threaded Executor) that opens the opportunity for Lock Elision. Additionally, this solution enables changing the priority of the executor thread and further speed up the resource group operations.

Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* ...
* ...

Hive Changes
* ...
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

@rmarduga rmarduga force-pushed the single-threaded-resource-group-ops branch 3 times, most recently from 5375467 to 70cf4f5 Compare November 23, 2022 14:21
@rmarduga rmarduga force-pushed the single-threaded-resource-group-ops branch from 70cf4f5 to 5020a12 Compare December 16, 2022 15:12
Problem description
Presently, all operations on Resource Group tree-like data
structure are protected by the root group monitor. That
makes all operation are blockable. Essentially, these
operations are the bottleneck for the Coordinator. It limits the
number of queries that could be admitted by Coordinator.
Additionally, those operations are invoked from many different
threads that could be scheduled on different cores. That
hurts the performance of those operations due to the
necessity of running costly cache coherence operations to
invalidate local caches on different hardware cores. That
limits the bandwidth on operations over Resource Groups.

Proposed solution
Rewrite the InternalResourceGroup class making all the
operations on resource group tree to be executed in the
Single Threaded Executor. After moving this logic into
executor, all operation will be executed in a single thread that
opens the opportunity for Lock Elusion.
@rmarduga rmarduga force-pushed the single-threaded-resource-group-ops branch from 5020a12 to bee63d4 Compare December 19, 2022 15:18
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

1 participant