Feature/4.3.0 backports#13
Merged
Merged
Conversation
A variety of Iceberg statements (including v2 deletes) rely on getting information from the scan node child of the delete node. Since tuple caching can insert a TupleCacheNode above that scan, the logic is currently failing, because it doesn't know how to bypass the TupleCacheNode and get to the scan node below. This modifies the logic in multiple places to detect a TupleCacheNode and go past it to the get the scan node below it. Testing: - Added a basic Iceberg test with v2 deletes for the frontend test and custom cluster test Change-Id: I162e738c4e4449a536701a740272aaac56ce8fd8 Reviewed-on: http://gerrit.cloudera.org:8080/22666 Reviewed-by: Kurt Deschler <kdeschle@cloudera.com> Reviewed-by: Michael Smith <michael.smith@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
…nline views
Some queries involving plain (distinct) UNIONs miss conjuncts, leading
to incorrect results:
Example:
WITH u1 AS (select 10 a, 10 b),
t AS (select a, b, min(b) over (partition by a) min_b from u1 UNION
select 10, 10, 20)
select t.* from t where t.b = t.min_b;
Expected result:
+----+----+-------+
| a | b | min_b |
+----+----+-------+
| 10 | 10 | 10 |
+----+----+-------+
Actual result:
+----+----+-------+
| a | b | min_b |
+----+----+-------+
| 10 | 10 | 10 |
| 10 | 20 | 10 |
+----+----+-------+
This is caused by MultiAggregateInfo assuming that conjuncts bound by
grouping slots that are produced by SlotRef grouping expressions are
already evaluated below the AggregationNode. However, this is not true
in all cases: with UNIONs, there may be conjuncts that are unassigned
below the AggregationNode.
This may happen if a conjunct cannot be pushed into all operands of a
UNION, because the source tuples in the operands do not contain all of
the slots referenced by the predicate. In the example above, it happens
in the first operand:
select a, b, min(b) over (partition by a) min_b from u1
The source tuple, 'u1', contains only two slots ('a' and 'b'), but does
not contain a slot corresponding to 'min(b)' - therefore the predicate
't.b = t.min_b' is not bound by the tuple of 'u1'. In theory, the
predicate could still be evaluated directly after materialising the
tuple with 'min(b)', still inside the UNION operand, but Impala
currently does not work that way.
In these cases, the conjuncts need to be evaluated in the
AggregationNode (possibly in addition to some of the UNION operands).
This change fixes this problem by introducing a method in
MultiAggregateInfo: 'setConjunctsToKeep()', where the caller can pass a
list of conjuncts that will not be eliminated. This is called during the
planning of the UNION if there are unassigned conjuncts remaining.
Testing:
- Added a PlannerTest and an EE test for the case where a conjunct
was previously incorrectly removed from the AggregationNode.
- Existing tests cover the case when conjuncts can be safely removed
from an AggregationNode above a UnionNode because the conjuncts are
pushed into all union operands, see for example
https://github.com/apache/impala/blob/6f2d9a2/testdata/workloads/functional-planner/queries/PlannerTest/union.test#L3914
Change-Id: I67a59cd96d83181ce249fd6ca141906f549a09b3
Reviewed-on: http://gerrit.cloudera.org:8080/22746
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
IMPALA-11604 (part 2) changes how many instances to create in Scheduler::CreateInputCollocatedInstances. This works when the left child fragment of a parent fragment is distributed across nodes. However, if the left child fragment instance is limited to only 1 node (the case of UNPARTITIONED fragment), the scheduler might over-parallelize the parent fragment by scheduling too many instances in a single node. This patch attempts to mitigate the issue in two ways. First, it adds bounding logic in PlanFragment.traverseEffectiveParallelism() to lower parallelism further if the left (probe) side of the child fragment is not well distributed across nodes. Second, it adds TQueryExecRequest.max_parallelism_per_node to relay information from Analyzer.getMaxParallelismPerNode() to the scheduler. With this information, the scheduler can do additional sanity checks to prevent Scheduler::CreateInputCollocatedInstances from over-parallelizing a fragment. Note that this sanity check can also cap MAX_FS_WRITERS option under a similar scenario. Added ScalingVerdict enum and TRACE log it to show the scaling decision steps. Testing: - Add planner test and e2e test that exercise the corner case under COMPUTE_PROCESSING_COST=1 option. - Manually comment the bounding logic in traverseEffectiveParallelism() and confirm that the scheduler's sanity check still enforces the bounding. Change-Id: I65223b820c9fd6e4267d57297b1466d4e56829b3 Reviewed-on: http://gerrit.cloudera.org:8080/22840 Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This extends tuple caching to be able to cache above joins. As part of this, ExchangeNodes are now eligible for broadcast and directed exchanges. This does not yet support partitioned exchanges. Since an exchange passes data from all nodes, this incorporates all the scan range information when passing through an exchange. For joins with a separate build side, a cache hit above the join means that a probe-side thread will never arrive. If the builder is not notified, it will wait for that thread to arrive and extend the latency of the query significantly. This adds code to notify the builder when a thread will never participate in the probe phase. Testing: - Added test cases to TestTupleCace including with distributed plans. - Added test cases to test_tuple_cache.py to verify behavior when updating the build side table and the timing of a cache hit. - Performance tests with TPC-DS at scale Change-Id: Ic61462702b43175c593b34e8c3a14b9cfe85c29e Reviewed-on: http://gerrit.cloudera.org:8080/22371 Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
With mt_dop, IcebergDeleteNode has the same independent builder as PartitionedHashJoinNode. It needs the same logic used for IMPALA-13660 to notify the build side when a probe side thread closes before probing. Interestingly enough, this is hard to demonstrate with a select. The coordinator cancels the query when it receives all the rows, which posts the builder out of its wait. However, for a delete, this is not true, so it can hang indefinitely. This centralizes the necessary logic to share it between PartitionedHashJoinNode, NestedLoopJoinNode, and IcebergDeleteNode. Applying it to IcebergDeleteNode fixes the hang. Testing: - Added a test case that consistently reproduced the hang before the fix Change-Id: Iff9228446f69ce43ed303c96893a91b99474800d Reviewed-on: http://gerrit.cloudera.org:8080/24279 Reviewed-by: Yida Wu <wydbaggio000@gmail.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This patch updates log4j-core from 2.18.0 to 2.25.4. Change-Id: Icdf7357dbf7edbb60cb3374094f210cbfeea2744 Reviewed-on: http://gerrit.cloudera.org:8080/24301 Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com> Reviewed-by: Michael Smith <michael.smith@cloudera.com> Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
…tion Time-travel queries could throw UnsupportedOperationException when they found files replicated on data nodes that were not in the host index of the table. It's because ListMap's populate() method did not copy the given list, but wrapped the original list via Collections.synchronizedList(). When the given list was immutable then we got the above error when we wanted to extend the host index (during time-travel). This patch fixes ListMap's populate() method to copy the given list. Testing * unit tests added for ListMap * e2e tests added for Iceberg tables + time-travel Change-Id: I3773eb7a37e9918501bfa8a22707967e79024aca Reviewed-on: http://gerrit.cloudera.org:8080/24294 Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
BINARY data in text files are expected to be Base64 encoded. TextConverter::WriteSlot has a bug when it decodes base64 code, it does not set the NULL-indicator bit to NULL for the slots of the invalid BINARY values. Therefore later Tuple::CopyStrings can try to copy invalid StringValue objects. This patch fixes TextConverter::WriteSlot to set the NULL-indicator bit in case of Base64 parse errors. Testing * e2e test added Change-Id: I79b712e2abe8ce6ecfbce508fd9e4e93fd63c964 Reviewed-on: http://gerrit.cloudera.org:8080/22721 Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
No description provided.