korlov42 commented on code in PR #4195:
URL: https://github.com/apache/ignite-3/pull/4195#discussion_r1713784336
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/largecluster/LargeClusterFactory.java:
##########
@@ -77,8 +77,11 @@ public ExecutionTarget
partitioned(List<TokenizedAssignments> assignments) {
BitSet nodes = new BitSet(assignment.nodes().size());
for (Assignment a : assignment.nodes()) {
int node = nodeNameToId.getOrDefault(a.consistentId(), -1);
- assert node >= 0 : "invalid node";
- nodes.set(node);
+
+ // TODO Ignore unknown node until IGNITE-22969
+ if (node != -1) {
+ nodes.set(node);
+ }
}
partitionNodes[idx] = nodes;
Review Comment:
let's add validation, that `nodes` set contains at least one node. We can't
just ignore partition if all hosting nodes unavailable. Besides, we need to
extend tests to cover both cases: partition partially available and partition
is not available at all.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/largecluster/LargeClusterFactory.java:
##########
@@ -112,9 +115,11 @@ private BitSet nodeListToMap(List<String> nodes) {
for (String name : nodes) {
int id = nodeNameToId.getOrDefault(name, -1);
- assert id >= 0 : "invalid node";
- nodesMap.set(id);
+ // TODO Ignore unknown node until IGNITE-22969
Review Comment:
method `nodeListToMap` is used to create `oneOf`, `someOf`, `allOf` targets.
Former two are tolerant to missing nodes (but there must be at least one),
while later must be executed on _all_ provided nodes (that is exactly semantic
of `all of`).
btw, why did you changed this in the first place?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
