-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Refactor fault tolerant scheduler components #14069
Refactor fault tolerant scheduler components #14069
Conversation
ec6845a
to
7577495
Compare
core/trino-main/src/main/java/io/trino/execution/scheduler/FaultTolerantPartitioningScheme.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/execution/scheduler/FaultTolerantPartitioningScheme.java
Show resolved
Hide resolved
HostAddress requiredAddress = bucketNodeMap.getAssignedNode(split).getHostAndPort(); | ||
int partition = sourcePartitioningScheme.getPartition(split); | ||
Optional<InternalNode> assignedNode = sourcePartitioningScheme.getNodeRequirement(partition); | ||
if (assignedNode.isPresent()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this check now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the buckets are always pre-assigned to nodes (this is how NodePartitioningManager
works). This is a problem for fault tolerant execution as it limits the number of partitions (sets it equal to the number of nodes). In the future we need to resolve this issue. Once the NodePartitioningManager
no longer pre-assignes the nodes to buckets there will be no more partition-to-node mapping available. I wanted to make it future proof to avoid another refactoring.
7577495
to
aa990e1
Compare
@losipiuk Updated |
To encapsulate partition assignment logic
aa990e1
to
48ea533
Compare
The memory requirement might get readjusted while TaskDescriptor is supposed to be immutable
48ea533
to
82be803
Compare
Benchmarks are fine:
Detailed: https://gist.github.com/arhimondr/407eeaf2174949b0ef3e4b75c1a095b6 |
Description
Multiple refactors to prepare the components for the upcoming major scheduler refactor
Non-technical explanation
N/A
Release notes
(X) This is not user-visible and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: