You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2023/08/01 13:01:34 UTC
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20295
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in DISTRIBUTED mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we don't need to update the
IcebergDeleteState which tracks the state of the merge join.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs getting unordered.
This patch adds a fix to quickly reset the state of the merge join when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/bin/create-load-data.sh
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
4 files changed, 64 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/1
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 17:47:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 5:
(5 comments)
Thanks for the comments!
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332: *probe_pos - current_probe_pos_
> If I understand what 'current_probe_pos_' and '*probe_pos' means I think th
Ouch, good catch.
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332: const int64_t step = *probe_pos - current_probe_pos_;
> This 'step' is only used for checking if we process the next row ID that is
I introduced a bool, but for readability, I also kept 'step'.
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh@436
PS4, Line 436: # TODO: Why is there a REMOTE_LOAD condition? See IMPALA-4347
> Just wondering if this step could be added to functional_schema_template.sq
Thanks, this made me realize I can fix the original iceberg_lineitem_multiblock table, so I don't need to add a new one.
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3719
PS4, Line 3719: ---- CREATE_HIVE
> Is there a way to load the necessary file into this location without runnin
Removed this.
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3722
PS4, Line 3722: t2.test_id c3, min(t1.bigint_col) min_bigint, min(t2.test_zip) min_zip
> Nice! The branch new CONVERT TO ICEBERG in action :)
Yeah, but I also had to remove this as this table became redundant. :)
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 11:25:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 12:40:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9546/
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 15:13:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4: Code-Review+1
Thanks for fixing this, LGTM
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 05:51:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, Gergely Fürnstáhl, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20295
to look at the new patch set (#5).
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in BROADCAST mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we rely on the previous delete
updating IcebergDeleteState to the next deleted row id and skip the
binary search if it's greater than or equal to the current probe row id.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs being unordered.
This patch adds a fix to ignore presumptions and do a binary search when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
3 files changed, 31 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/5
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9553/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 11:24:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
LGTM, feel free to carry over
http://gerrit.cloudera.org:8080/#/c/20295/5/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/5/be/src/exec/iceberg-delete-node.cc@332
PS5, Line 332: probe_pos
Maybe we could rename this guy to next_probe_pos, for better readability.
In theory, this could cause an UB (integer overflow. I wonder if UBSAN build would detect it). At that point we have bigger problems to worry about, but not too hard to fix with different ordering:
bool is_consecutive_pos = false;
if(current_probe_pos_ != INVALID_ROW_ID) {
const step = *next_probe_pos - current_probe_pos_;
is_consecutive_pos = step == 1;
}
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 11:45:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 22:26:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 5: Code-Review+1
Apart from what Gergo found with the integer overflow I'm fine with this.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 11:57:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, Gergely Fürnstáhl, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20295
to look at the new patch set (#6).
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in BROADCAST mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we rely on the previous delete
updating IcebergDeleteState to the next deleted row id and skip the
binary search if it's greater than or equal to the current probe row id.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs being unordered.
This patch adds a fix to ignore presumptions and do a binary search when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
3 files changed, 37 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/6
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9557/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 17:47:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13680/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 13:27:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9547/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 15:21:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, Gergely Fürnstáhl, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20295
to look at the new patch set (#4).
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in BROADCAST mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we rely on the previous delete
updating IcebergDeleteState to the next deleted row id and skip the
binary search if it's greater than or equal to the current probe row id.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs being unordered.
This patch adds a fix to ignore presumptions and do a binary search when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/bin/create-load-data.sh
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
5 files changed, 70 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/4
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332: current_probe_pos_ - *probe_pos
If I understand what 'current_probe_pos_' and '*probe_pos' means I think this should be '*probe_pos - current_probe_pos' instead.
http://gerrit.cloudera.org:8080/#/c/20295/4/be/src/exec/iceberg-delete-node.cc@332
PS4, Line 332: const int64_t step = current_probe_pos_ - *probe_pos;
This 'step' is only used for checking if we process the next row ID that is right after the one we last processed, right?
If yes, then this can be a bool, I think.
e.g.
bool isConsecutivePos = current_probe_pos == 1 + *probe_pos;
Not sure about the name of the variable, though.
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/bin/create-load-data.sh@436
PS4, Line 436: # IMPALA-12327: Add multiblock test for Iceberg tables.
Just wondering if this step could be added to functional_schema_template.sql? I do see hadoop mkdir and command there as well.
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3719
PS4, Line 3719: LIKE PARQUET '/test-warehouse/lineitem_sixblocks_iceberg/lineitem_sixblocks.parquet'
Is there a way to load the necessary file into this location without running create-load-data? Or is there a way to run create-load-data that only runs the custom-pre-load-steps that is needed for this table?
http://gerrit.cloudera.org:8080/#/c/20295/4/testdata/datasets/functional/functional_schema_template.sql@3722
PS4, Line 3722: ALTER TABLE {db_name}{db_suffix}.{table_name} CONVERT TO ICEBERG;
Nice! The branch new CONVERT TO ICEBERG in action :)
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 08:34:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 6:
(1 comment)
Thanks for the comment!
http://gerrit.cloudera.org:8080/#/c/20295/5/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/5/be/src/exec/iceberg-delete-node.cc@332
PS5, Line 332: s = false
> Maybe we could rename this guy to next_probe_pos, for better readability.
Yeah, it's not too likely to hit an int64 overflow here, but fixed it anyway.
Clang's UB sanitizer is certainly capable of catching all kinds of overflows, but I'm unsure if we configure every checks.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 12:22:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13690/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 12:39:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9554/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 12:40:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13681/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 16:22:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13682/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 16:32:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 7: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9554/
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 16:55:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20295
to look at the new patch set (#2).
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in DISTRIBUTED mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we don't need to update the
IcebergDeleteState which tracks the state of the merge join.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs getting unordered.
This patch adds a fix to quickly reset the state of the merge join when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/bin/create-load-data.sh
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
4 files changed, 63 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/2
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in BROADCAST mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we rely on the previous delete
updating IcebergDeleteState to the next deleted row id and skip the
binary search if it's greater than or equal to the current probe row id.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs being unordered.
This patch adds a fix to ignore presumptions and do a binary search when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Reviewed-on: http://gerrit.cloudera.org:8080/20295
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/iceberg-delete-node.cc
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
3 files changed, 37 insertions(+), 9 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 9
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@10
PS2, Line 10: DISTRIBUTED
BROADCAST
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@13
PS2, Line 13: we don't need to update the
: IcebergDeleteState which tracks the state of the merge join.
we rely on the previous delete updating IcebergDeleteState to the next deleted row id and skip the binary search if it's greater than or equal to the current probe row id.
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@22
PS2, Line 22: getting
being
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@24
PS2, Line 24: reset
It's not really a reset, maybe:
This patch adds a fix to ignore presumptions and do a binary search when the ...
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@332
PS2, Line 332: int64_t step = current_probe_pos_ - *probe_pos;
nit: could be const
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@348
PS2, Line 348: So we are always doing a binary
: // search here to find the proper delete row id.
We could improve this case if we store the deleted row ids in an std::unordered_set too and switching to use that at the first case of step!=1 for O(1) lookups at the cost of slightly more memory. We could create a followup jira too, or just skip this and focus on directed distribution mode.
http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql@3701
PS2, Line 3701: iceberg_lineitem_multiblock
I meant to test this case with iceberg_lineitem_multiblock, seems like I messed up something during creation, we might as well delete this
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 15:39:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9548/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 18:01:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4:
(7 comments)
Thanks for the comments, Gergo!
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@10
PS2, Line 10: BROADCAST m
> BROADCAST
Done
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@13
PS2, Line 13: we rely on the previous delete
: updating IcebergDeleteState to the next deleted row id and s
> we rely on the previous delete updating IcebergDeleteState to the next dele
Done
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@22
PS2, Line 22: LHS coa
> being
Done
http://gerrit.cloudera.org:8080/#/c/20295/2//COMMIT_MSG@24
PS2, Line 24:
> It's not really a reset, maybe:
Done
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc
File be/src/exec/iceberg-delete-node.cc:
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@332
PS2, Line 332: const int64_t step = current_probe_pos_ - *probe_pos;
> nit: could be const
Done
http://gerrit.cloudera.org:8080/#/c/20295/2/be/src/exec/iceberg-delete-node.cc@348
PS2, Line 348: So we are always doing a binary
: // search here to find the proper delete row id.
> We could improve this case if we store the deleted row ids in an std::unord
Yeah I think we should focus on the DIRECTED distribution mode which won't have these problems.
http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:
http://gerrit.cloudera.org:8080/#/c/20295/2/testdata/datasets/functional/functional_schema_template.sql@3701
PS2, Line 3701: iceberg_lineitem_multiblock
> I meant to test this case with iceberg_lineitem_multiblock, seems like I me
The missing parts are in create-load-data.sh which sets the dfs.block.size for the file. Without that we only have lots of row groups in the Parquet file, but at HDFS level only one block.
Anyway, I think it's OK to keep it, as it doesn't hurt, and might still add some extra coverage for the case when there are lots of row groups in the file.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 16:10:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13689/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 11:45:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 12:39:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9546/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 13:13:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 4: Code-Review+2
Thank you for this fix Zoltan!
LGTM!
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 08:00:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Aug 2023 21:50:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20295 )
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13679/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Tue, 01 Aug 2023 13:25:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Tamas Mate, Gabor Kaszab, Gergely Fürnstáhl, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20295
to look at the new patch set (#3).
Change subject: IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
......................................................................
IMPALA-12327: Iceberg V2 operator wrong results in PARTITIONED mode
The Iceberg delete node tries to do mini merge-joins between data
records and delete records. This works in DISTRIBUTED mode, and most of
the time in PARTITIONED mode as well. Though the Iceberg delete node had
the wrong assumption that if the rows in a row batch belong to the same
file, and come in ascending order, we don't need to update the
IcebergDeleteState which tracks the state of the merge join.
When PARTITIONED mode is used, we cannot rely on ascending row order,
not even inside row batches, not even when the previous file path is the
same as the current one. This is because files with multiple blocks can
be processed by multiple hosts in parallel, then the rows are getting
hash-exchanged based on their file paths. Then the exchange-receiver at
the LHS coalesces the row batches from multiple senders, hence the row
IDs getting unordered.
This patch adds a fix to quickly reset the state of the merge join when
the position-based difference between the current row and previous row
is not one, and we are in PARTITIONED mode.
Tests:
* added e2e tests
Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
---
M be/src/exec/iceberg-delete-node.cc
M testdata/bin/create-load-data.sh
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-read-position-deletes.test
5 files changed, 70 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/20295/3
--
To view, visit http://gerrit.cloudera.org:8080/20295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib89a53e812af8c3b8ec5bc27bca0a50dcac5d924
Gerrit-Change-Number: 20295
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <g....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>