Move local parallel to planner #4699
Conversation
|
The first commit |
|
@nileema I updated this to address all of @erichwang's comments in the planner parts. |
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| @ThreadSafe | ||
| public class LocalExchange |
There was a problem hiding this comment.
@haozhun can you review this and the related classes for any possible concurrency issues?
dain
force-pushed
the
local-exchanges
branch
2 times, most recently
from
a1cdab7 to
73d14ea
Compare
|
@dain did I miss something, or you didn't respond to my comments/questions? Now I'am unable to even find my comments. You could create new pull request after you recent changes instead of overwriting this one :( Ok, I was able to recover hashes for some of my comments that disappeared: |
|
Add partial flag to LimitNode looks good. |
|
Remove experimental use-intermediate-aggregations feature Look good |
| synchronized (this) { | ||
| allSourcesFinished = true; | ||
|
|
||
| // since all sources are finished there is no reason to all new pages |
| case LEFT: | ||
| // the left can contain nulls in any stream so we can't say anything about the | ||
| // partitioning but the other properties of the left will be maintained. | ||
| return leftProperties.withoutSymbols(); |
There was a problem hiding this comment.
Maybe call this withUnspecifiedPartitioning or withArbitraryPartitioning?
Add local exchanges during planning to increase parallelism in tasks.
Local parallelism is planned in AddLocalExchanges so unplanned, automatic parallelism in LocalExecutionPlanner is no longer needed. Remove InMemoryExchange which has been superseded by LocalExchange
build outer joins.
instead of adding extra projection