You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "lowka (via GitHub)" <gi...@apache.org> on 2023/07/21 13:23:04 UTC
[GitHub] [ignite-3] lowka opened a new pull request, #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
lowka opened a new pull request, #2342:
URL: https://github.com/apache/ignite-3/pull/2342
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275192117
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -820,6 +830,27 @@ private void enlist(SourceAwareIgniteRel rel) {
}.visit(fragment.root());
}
+ private CompletableFuture<MultiStepPlan> mapFragments(MultiStepPlan plan) {
+ List<IgniteRel> fragments = plan.fragments()
+ .stream()
+ .map(Fragment::root)
+ .collect(Collectors.toList());
Review Comment:
Sure. I thought of it but I forgot about `TransformingIterator`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275180105
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -952,4 +983,30 @@ public interface ImplementorFactory<RowT> {
/** Creates the relational node implementor with the given context. */
LogicalRelImplementor<RowT> create(ExecutionContext<RowT> ctx, ResolvedDependencies resolvedDependencies);
}
+
+ static final class FetchColocationGroups {
+
+ private final ResolvedDependencies deps;
+
+ FetchColocationGroups(ResolvedDependencies deps) {
+ this.deps = deps;
+ }
+
+ CompletableFuture<Map<Integer, ColocationGroup>> execute() {
+ List<CompletableFuture<Pair<Integer, ColocationGroup>>> list = new ArrayList<>();
+
+ for (Integer tableId : deps.tableIds()) {
+ CompletableFuture<ColocationGroup> f = deps.fetchColocationGroup(tableId);
+ list.add(f.thenApply(c -> new Pair<>(tableId, c)));
+ }
+
+ CompletableFuture<Void> all = CompletableFuture.allOf(list.toArray(new CompletableFuture[0]));
+
+ return all.thenApply(
+ v -> list.stream()
+ .map(CompletableFuture::join)
+ .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)));
+
+ }
+ }
Review Comment:
Seems, this method can be static helper method and class can be dropped.
Or maybe the whole method can be moved to `ResolvedDependencies` class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov merged pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov merged PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275208516
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -820,6 +830,27 @@ private void enlist(SourceAwareIgniteRel rel) {
}.visit(fragment.root());
}
+ private CompletableFuture<MultiStepPlan> mapFragments(MultiStepPlan plan) {
+ List<IgniteRel> fragments = plan.fragments()
+ .stream()
+ .map(Fragment::root)
+ .collect(Collectors.toList());
Review Comment:
Added `TransformingIterator::newIterable`. And replaced `List<IgniteRel>` with `Iterable<IgniteRel>` in DepedencyResolver's signature.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -952,4 +983,30 @@ public interface ImplementorFactory<RowT> {
/** Creates the relational node implementor with the given context. */
LogicalRelImplementor<RowT> create(ExecutionContext<RowT> ctx, ResolvedDependencies resolvedDependencies);
}
+
+ static final class FetchColocationGroups {
+
+ private final ResolvedDependencies deps;
+
+ FetchColocationGroups(ResolvedDependencies deps) {
+ this.deps = deps;
+ }
+
+ CompletableFuture<Map<Integer, ColocationGroup>> execute() {
+ List<CompletableFuture<Pair<Integer, ColocationGroup>>> list = new ArrayList<>();
+
+ for (Integer tableId : deps.tableIds()) {
+ CompletableFuture<ColocationGroup> f = deps.fetchColocationGroup(tableId);
+ list.add(f.thenApply(c -> new Pair<>(tableId, c)));
+ }
+
+ CompletableFuture<Void> all = CompletableFuture.allOf(list.toArray(new CompletableFuture[0]));
+
+ return all.thenApply(
+ v -> list.stream()
+ .map(CompletableFuture::join)
+ .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)));
+
+ }
+ }
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275198490
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -820,6 +830,27 @@ private void enlist(SourceAwareIgniteRel rel) {
}.visit(fragment.root());
}
+ private CompletableFuture<MultiStepPlan> mapFragments(MultiStepPlan plan) {
+ List<IgniteRel> fragments = plan.fragments()
+ .stream()
+ .map(Fragment::root)
+ .collect(Collectors.toList());
Review Comment:
Well, here is a problem it is a bad idea to accept an iterator because is modifiable. We need something like transforming iterable here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275135241
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -820,6 +830,27 @@ private void enlist(SourceAwareIgniteRel rel) {
}.visit(fragment.root());
}
+ private CompletableFuture<MultiStepPlan> mapFragments(MultiStepPlan plan) {
+ List<IgniteRel> fragments = plan.fragments()
+ .stream()
+ .map(Fragment::root)
+ .collect(Collectors.toList());
Review Comment:
Does it make sense to avoid collection copying?
We could use smth like `org.apache.ignite.internal.utilTransformingIterator`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2342: IGNITE-18453: Query executor thread synchronously waits for leaders on query mapping.
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2342:
URL: https://github.com/apache/ignite-3/pull/2342#discussion_r1275195987
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##########
@@ -952,4 +983,30 @@ public interface ImplementorFactory<RowT> {
/** Creates the relational node implementor with the given context. */
LogicalRelImplementor<RowT> create(ExecutionContext<RowT> ctx, ResolvedDependencies resolvedDependencies);
}
+
+ static final class FetchColocationGroups {
+
+ private final ResolvedDependencies deps;
+
+ FetchColocationGroups(ResolvedDependencies deps) {
+ this.deps = deps;
+ }
+
+ CompletableFuture<Map<Integer, ColocationGroup>> execute() {
+ List<CompletableFuture<Pair<Integer, ColocationGroup>>> list = new ArrayList<>();
+
+ for (Integer tableId : deps.tableIds()) {
+ CompletableFuture<ColocationGroup> f = deps.fetchColocationGroup(tableId);
+ list.add(f.thenApply(c -> new Pair<>(tableId, c)));
+ }
+
+ CompletableFuture<Void> all = CompletableFuture.allOf(list.toArray(new CompletableFuture[0]));
+
+ return all.thenApply(
+ v -> list.stream()
+ .map(CompletableFuture::join)
+ .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)));
+
+ }
+ }
Review Comment:
Sure. This class can be replaced with a simple function that does exactly what execute does.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org