Make scheduling of remote accessible splits with addresses more strict #22190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
losipiuk
approved these changes
May 29, 2024
core/trino-main/src/main/java/io/trino/execution/scheduler/UniformNodeSelector.java
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/execution/scheduler/UniformNodeSelector.java
Show resolved
Hide resolved
sopel39
force-pushed
the
ks/remote_accessible
branch
from
bf5f078 to
445636a
Compare
raunaqmorarka
approved these changes
May 30, 2024
dekimir
reviewed
May 31, 2024
...main/java/io/trino/execution/scheduler/faulttolerant/ArbitraryDistributionSplitAssigner.java
Show resolved
Hide resolved
...src/main/java/io/trino/execution/scheduler/faulttolerant/BinPackingNodeAllocatorService.java
Show resolved
Hide resolved
sopel39
force-pushed
the
ks/remote_accessible
branch
from
445636a to
215e0dc
Compare
martint
approved these changes
Jun 5, 2024
sopel39
force-pushed
the
ks/remote_accessible
branch
from
215e0dc to
2c5c0e6
Compare
sopel39
commented
Jun 10, 2024
| @@ -139,7 +139,7 @@ public AssignmentResult assign(PlanNodeId planNodeId, ListMultimap<Integer, Spli | |||
| .orElse(ImmutableSet.of()); | |||
| assignment.addPartition(new Partition( | |||
| taskPartitionId, | |||
| new NodeRequirements(catalogRequirement, hostRequirement))); | |||
| new NodeRequirements(catalogRequirement, hostRequirement, true))); | |||
| @@ -83,7 +83,7 @@ public AssignmentResult finish() | |||
| if (!partitionAdded) { | |||
| partitionAdded = true; | |||
| result | |||
| .addPartition(new Partition(0, new NodeRequirements(Optional.empty(), hostRequirement))) | |||
| .addPartition(new Partition(0, new NodeRequirements(Optional.empty(), hostRequirement, true))) | |||
sopel39
force-pushed
the
ks/remote_accessible
branch
from
2c5c0e6 to
48841b2
Compare
UniformNodeSelector or FTE scheduler will schedule remote accessible splits on selected nodes if such nodes are available and only fallback to other nodes if preferred nodes are no longer part of cluster. Connector might have stale node information when creating splits which could result in choosing offline nodes. Additionally, in FTE mode nodes can go down so split addresses could no longer be valid then task is restarted. Additionally, this commit simplifies UniformNodeSelector optimizedLocalScheduling which was hard to reason about and was not taking advantages of recent improvements like adaptive split queue length. Co-authored-by: Karol Sobczak <[email protected]>
sopel39
force-pushed
the
ks/remote_accessible
branch
from
48841b2 to
8e8ca54
Compare
|
We don't really document that there are selected nodes as part of FTE... so I'm not sure how user-visible this change would be or if a release note is necessary? |
It's more for devs, so probably should be in SPI section |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
UniformNodeSelector or FTE scheduler will schedule remote accessible splits on selected nodes if such nodes are available and only fallback to other nodes is nodes are no longer part of cluster. Connector might have stalled node information while creating splits which could result in selecting nodes which are now offline. Additionally, in FTE mode nodes can go down so split addresses could no longer be valid then task is restarted.
Additionally, this commit simplifies UniformNodeSelector optimizedLocalScheduling which was hard to reason about and was not taking advantages of recent improvements like adaptive split queue length.
Extracted from: #21888
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: