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

Configure worker threads relative to core count #20772

Conversation

findepi
Copy link
Member

@findepi findepi commented Feb 20, 2024

Worker threads defaults to 2*#cores. This commit allows configuration to be #cores based too.

Extracted from #16303

Worker threads defaults to 2*#cores. This commit allows configuration to
be #cores based too.

Extracted from trinodb#16303

Co-authored-by: Mateusz "Serafin" Gajewski <[email protected]>
Copy link
Member

@raunaqmorarka raunaqmorarka left a comment

Choose a reason for hiding this comment

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

.

@@ -286,9 +286,9 @@ public int getMaxWorkerThreads()

@LegacyConfig("task.shard.max-threads")
@Config("task.max-worker-threads")
public TaskManagerConfig setMaxWorkerThreads(int maxWorkerThreads)
public TaskManagerConfig setMaxWorkerThreads(String maxWorkerThreads)
Copy link
Member

Choose a reason for hiding this comment

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

Can we add @ConfigDescription and documentation ?
Maybe also deprecate task.shard.max-threads in another commit

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe also deprecate task.shard.max-threads in another commit

that's orthogonal, i believe?

@oneonestar
Copy link
Member

I think it should accept decimal number for better configurability.

$ ./mvnw -T 1.5C compile
[INFO] Scanning for projects...
[INFO]
[INFO] Using the MultiThreadedBuilder implementation with a thread count of 15

@findepi
Copy link
Member Author

findepi commented Feb 21, 2024

I think it should accept decimal number for better configurability.

i didn't have a use-case for this. note that effective configuration should be exact, whole number of threads, so we don't want floating point arithmetics semantics.

@findepi findepi merged commit 89d4037 into trinodb:master Feb 21, 2024
93 checks passed
@findepi findepi deleted the findepi/configure-worker-threads-relative-to-core-count-144a9e branch February 21, 2024 08:48
@github-actions github-actions bot added this to the 440 milestone Feb 21, 2024
arhimondr added a commit to arhimondr/presto that referenced this pull request May 22, 2024
arhimondr added a commit to arhimondr/presto that referenced this pull request May 24, 2024
arhimondr added a commit to arhimondr/presto that referenced this pull request May 24, 2024
arhimondr added a commit to arhimondr/presto that referenced this pull request May 24, 2024
arhimondr added a commit to arhimondr/presto that referenced this pull request May 28, 2024
arhimondr added a commit to prestodb/presto that referenced this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants