You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2023/06/30 09:39:59 UTC
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20145
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M tests/custom_cluster/test_events_custom_configs.py
2 files changed, 47 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/1
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
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/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Reviewed-on: http://gerrit.cloudera.org:8080/20145
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
5 files changed, 85 insertions(+), 8 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
LGTM +1. Just a minor question.
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py@847
PS2, Line 847: "alter table %s.part partition(p=0) set tblproperties('k'='v')" % unique_database
Should we try compaction also here?
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 21:26:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@858
PS6, Line 858: self.client.execute(
: "alter table %s.part drop partition (p=0)" % unique_database)
: self.client.execute(
: "insert into %s.part partition(p=0) values (1),(2)" % unique_database)
: self.client.execute(ddl)
> Thanks for the detailed explanation Quanlong! :-)
Yeah, exactly. The key part is verifying no partitions will be added/dropped/refreshed by events. The other partition events (e.g. ADD_PARTITION) will also be skipped so "events-skipped" might increase more than one in each iteraton.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Jul 2023 13:50:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#3).
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
4 files changed, 59 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/3
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20145/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/20145/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3903
PS5, Line 3903: LOG.info("Partition {} of table {}.{} has last refresh id as {}. Comparing it with {}.",
line too long (94 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 08:01:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#6).
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
5 files changed, 63 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/6
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#4).
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
4 files changed, 60 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/4
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
Thanks for addressing my previous comments Quanlong!
I do not have any other suggestion.
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2904
PS4, Line 2904: partBuilder.setCreateEventId(oldPartition.getCreateEventId());
: partBuilder.setLastCompactionId(oldPartition.getLastCompactionId());
> 1. The lastRefreshEventId is set below at line 2907. Its source value is fe
Thanks for the explanation Quanlong!
Yeah. You are correct. We do not need to use the 'lastRefreshEventId_' from 'oldPartition' since 'latestEventId' is always higher/fresher and I just realized that even when we are processing a non-self INSERT event, we still update the field of 'lastRefreshEventId_' with the event ID of this INSERT event.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 20:57:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13465/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 23:46:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13443/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 10:02:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13464/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 23:42:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 14:09:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 18:20:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9475/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 00:39:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13470/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 08:28:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#5).
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
5 files changed, 63 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/5
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9480/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 08:05:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 4:
(1 comment)
> Patch Set 3:
>
> (1 comment)
>
> Thanks for the quick review! Fix losing create event ids in REFRESH.
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py@847
PS2, Line 847: "alter table %s.part partition(p=0) set tblproperties('k'='v')" % unique_database,
> Good point. However, these are executed in Impala. We haven't supported tri
Ack
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 01:02:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 3:
(1 comment)
Thanks for the quick review! Fix losing create event ids in REFRESH.
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/2/tests/custom_cluster/test_events_custom_configs.py@847
PS2, Line 847: "alter table %s.part partition(p=0) set tblproperties('k'='v')" % unique_database,
> Should we try compaction also here?
Good point. However, these are executed in Impala. We haven't supported triggering compactions yet. Neither do we support these ALTER statements on transactional tables.
Note that this test is to verify that DDLs in Impala won't lose the tracked create event ids in partition-level.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 23:24:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@858
PS6, Line 858: self.client.execute(
: "alter table %s.part drop partition (p=0)" % unique_database)
: self.client.execute(
: "insert into %s.part partition(p=0) values (1),(2)" % unique_database)
: self.client.execute(ddl)
> When we submit the 3 queries above to Impala, is it true that the order of the respective events received from HMS is non-deterministic?
No, the order is deterministic.
> What if we receive the DROP_PARTITION event prior to the events corresponding to the other 2 queries? Will this DROP_PARTITION event be skipped in this case? Or some logic ensures even in this case (the DROP_PARTITION event arriving first) this DROP_PARTITION will still be skipped?
We use a large polling interval (5s) in this test so it's more likely that event-processor fetched the events after the DDLs finish. Note that event-processor is scheduled in another independent thread. It's possible that it runs right after the first AlterTableDropPartition finishes and then gets the DROP_PARTITION event and processes it. In this case, the partition doesn't exist and the DROP_PARTITION event will be skipped as well. But the possibility is pretty small. In most of the cases, the event-processor will fetch events after these 3 DDLs finish. This test aims to verify that the DROP_PARTITION event is correctly skipped in this case.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 08 Jul 2023 08:14:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Fang-Yu Rao, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#7).
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
5 files changed, 85 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/7
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 12:18:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13445/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 11:45:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
(1 comment)
Thanks for the explanation Quanlong! I do not have other questions. :-)
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@858
PS6, Line 858: self.client.execute(
: "alter table %s.part drop partition (p=0)" % unique_database)
: self.client.execute(
: "insert into %s.part partition(p=0) values (1),(2)" % unique_database)
: self.client.execute(ddl)
> > When we submit the 3 queries above to Impala, is it true that the order o
Thanks for the detailed explanation Quanlong! :-)
Now it's much clearer to me as to why "events-skipped" should be increased even in the case in which only the first DDL (ALTER TABLE DROP PARTITION) was finished when the event processor receives from HMS the corresponding DROP_PARTITION event. In such a case, 'hdfsPartition'
within CatalogOpExecutor#canDropPartitionFromEvent() ( https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L4546) would be null and thus such a DROP_PARTITION event has to be skipped by the event processor.
Thus, whether or not the 2nd DDL (INSERT INTO PARTITION) is finished when the DROP_PARTITION event is received, such an event will be skipped.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Jul 2023 02:25:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
(1 comment)
Sorry I have another question that I forgot to ask in my previous review. I asked this question mostly for my own understanding. Thanks!
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@858
PS6, Line 858: self.client.execute(
: "alter table %s.part drop partition (p=0)" % unique_database)
: self.client.execute(
: "insert into %s.part partition(p=0) values (1),(2)" % unique_database)
: self.client.execute(ddl)
I have another question regarding the events corresponding to the 3 queries above.
When we submit the 3 queries above to Impala, is it true that the order of the respective events received from HMS is non-deterministic?
What if we receive the DROP_PARTITION event prior to the events corresponding to the other 2 queries? Will this DROP_PARTITION event be skipped in this case? Or some logic ensures even in this case (the DROP_PARTITION event arriving first) this DROP_PARTITION will still be skipped?
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 08 Jul 2023 01:29:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@846
PS6, Line 846: " delimited fields terminated by ','" % unique_database,
> Can you add a few more comments to the test? I found it a bit confusing. If
Sure. Added more comments.
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@858
PS6, Line 858: for ddl in partition_ddls:
: events_skipped_before = EventProcessorUtils.get_int_metric("events-skipped")
: # Drop-create the partition and then runs a DDL on it. A DROP_PARTITION and an
: # ADD_PARTITION event will be generated and should be skipped. The 3rd DDL might
: # generate an ALTER_PART
> Yeah, exactly. The key part is verifying no partitions will be added/droppe
Added some comments for this.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 12:26:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Sai Hemanth Gantasala, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20145
to look at the new patch set (#2).
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
When HMS event-processor is enabled, each partition will track the
event id of the ADD_PARTITION event that corresponds to its creation.
The create event id is used to skip stale DROP_PARTITION events that
have lower event ids.
However, the partition-level create event ids are not correctly retained
during DDLs. DDLs that modify the partition metadata will reload the
partitions. Since HdfsPartition objects are immutable, the reload is
performed as replacing the old one with the new object created by
HdfsPartition.Builder. The HdfsPartition.Builder is built from the old
HdfsPartition object. But it doesn't copy the create event id. This
patch fixes the issue, also adds logs for comparing partition event ids.
Tests:
- Add e2e tests to verify DDLs won't loss the partition-level create
event ids, so stale ADD_PARTITION events can be skipped as expected.
Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
---
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/custom_cluster/test_events_custom_configs.py
3 files changed, 56 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/20145/2
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jul 2023 04:49:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing partition-level create event ids in reloads
......................................................................
Patch Set 4: Code-Review+1
(3 comments)
Thanks for the analysis and the prompt fix Quanlong!
I only have some minor questions mostly because I know very little around this area.
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1326
PS4, Line 1326: loss
nit: Should we use "lose" instead?
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2904
PS4, Line 2904: partBuilder.setCreateEventId(oldPartition.getCreateEventId());
: partBuilder.setLastCompactionId(oldPartition.getLastCompactionId());
I have 2 questions.
1. At https://gerrit.cloudera.org/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java#1329 we also populate the field 'lastRefreshEventId_'. I wonder if we also need to pick up the value of 'lastRefreshEventId_' like the following. Or we do not really need to do this because we already populate the field 'lastRefreshEventId_' somewhere else and this field will be available when the event processor is processing a reload event (e.g., at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L3891)?
partBuilder.setLastRefreshEventId(oldPartition.getLastRefreshEventId());
2. Here we also pick up the value of 'lastCompactionId_'. Will this piece of information be required to fix the issue reported in this JIRA? Or we just want to keep this field in sync with the same field in the partition that was just updated by the DDL command?
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4544
PS4, Line 4544: // if the partition has been created since the event was generated, skip
: // the stale event.
: boolean isStale = hdfsPartition.getCreateEventId() > eventId;
: LOG.info("{} partition {} of table {} since it's create event id {} is {} than " +
: "eventid {}",
: isStale ? "Not dropping" : "Dropping",
: hdfsPartition.getPartitionName(), hdfsTable.getFullName(),
: hdfsPartition.getCreateEventId(), isStale ? "higher" : "not higher",
: eventId);
: return !isStale;
:
At https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L3891 we have the following check within CatalogServiceCatalog.java#isPartitionLoadedAfterEvent(). I wonder if it would also be good for us to log the reload event?
HdfsPartition hdfsPartition = getHdfsPartition(dbName, tableName, msPartition);
if (hdfsPartition.getLastRefreshEventId() > eventId) {
return true;
}
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sun, 02 Jul 2023 00:06:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13469/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 08:24:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6:
(3 comments)
Thank Fang-Yu's review!
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1326
PS4, Line 1326: lose
> nit: Should we use "lose" instead?
Done
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2904
PS4, Line 2904: partBuilder.setCreateEventId(oldPartition.getCreateEventId());
: partBuilder.setLastCompactionId(oldPartition.getLastCompactionId());
> I have 2 questions.
1. The lastRefreshEventId is set below at line 2907. Its source value is fetched at line 2889 which is always higher than the id in the old partition. So I think we don't need to set it here.
2. The compaction id is unrelated to this issue. I just fix it by the way. I think losing this id might cause other issues. But I'm not sure whether we already fix it in other places. I think populating the lastCompactionId is safe.
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/20145/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4544
PS4, Line 4544: // if the partition has been created since the event was generated, skip
: // the stale event.
: boolean isStale = hdfsPartition.getCreateEventId() > eventId;
: LOG.info("{} partition {} of table {} since it's create event id {} is {} than " +
: "eventid {}",
: isStale ? "Not dropping" : "Dropping",
: hdfsPartition.getPartitionName(), hdfsTable.getFullName(),
: hdfsPartition.getCreateEventId(), isStale ? "higher" : "not higher",
: eventId);
: return !isStale;
:
> At https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache
Yeah, sure. That would be good for debuggability.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 08:05:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13519/ : 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/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 12:48:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9492/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 14:10:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20145 )
Change subject: IMPALA-12256: Fix DDLs losing create event ids in reloading partitions
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/20145/6/tests/custom_cluster/test_events_custom_configs.py@846
PS6, Line 846: "alter table %s.part partition(p=0) set location '/tmp'" % unique_database,
Can you add a few more comments to the test? I found it a bit confusing. If I understand correctly some of these DDLs would lead to events that cannot be skipped if they were done by another component, but Impala will detect them as self event and skip them. Meanwhile the self event logic doesn't handle drop partitions. Probably it would be useful to add some notes on why different events are skipped.
--
To view, visit http://gerrit.cloudera.org:8080/20145
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052faa093bda69fb16db0d424c1478bba103dad9
Gerrit-Change-Number: 20145
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jul 2023 11:07:27 +0000
Gerrit-HasComments: Yes