[Impala-ASF-CR] IMPALA-12277: Fix NullPointerException for partitioned inserts when EP is turned off
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21437 )
Change subject: IMPALA-12277: Fix NullPointerException for partitioned inserts
when EP is turned off
..
Patch Set 13:
(7 comments)
Thanks for refactoring the long methods! Just took a first pass. I'll look
deeper into this.
http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG@7
PS14, Line 7: when
: EP is turned off
nit: let's make this accurate, e.g. "when partition list is stale"
http://gerrit.cloudera.org:8080/#/c/21437/13/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/21437/13/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1292
PS13, Line 1292: Preconditions.checkState(
nit: as we touch this code, can we add an error message for
Preconditions.checkState() BTW? E.g.
Preconditions.checkState(
partitionsToUpdate == null || loadPartitionFileMetadata,
"Conflicts in 'partitionsToUpdate' and
'loadPartitionFileMetadata'");
http://gerrit.cloudera.org:8080/#/c/21437/14/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/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1238
PS14, Line 1238: metadata should be updated. O
nit: we usually put the comment before the value, e.g. L1225-L1227
http://gerrit.cloudera.org:8080/#/c/21437/13/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/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7253
PS13, Line 7253: false
nit: could you add a comment to highlight this is 'loadPartitionFileMetadata'?
http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7279
PS13, Line 7279: updatePartitionsToCreateAndUnsetStats
nit: let's avoid using the variable name 'partitionsToCreate' in the method
name. Maybe 'pickupExistingPartitions' is enough. Other details like unsetting
COLUMN_STATS_ACCURATE and collecting cacheDirIds can be mentioned in the method
comments.
http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7405
PS13, Line 7405:
nit: remove one space here
http://gerrit.cloudera.org:8080/#/c/21437/13/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:
http://gerrit.cloudera.org:8080/#/c/21437/13/tests/custom_cluster/test_events_custom_configs.py@1275
PS13, Line 1275: self.client.execute("insert into {}.{}
partition(year=2024) values (0)"
Can we also add year=2022 as an existing partition? Now we have stale partition
(year=2024) dropped externally and missing partition (year=2023) added
externally. Just miss the case of inserting into existing partitions.
--
To view, visit http://gerrit.cloudera.org:8080/21437
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide8f1f6bf017e9a040b53bb5d5291ff2ea3e0d18
Gerrit-Change-Number: 21437
Gerrit-PatchSet: 13
Gerrit-Owner: Sai Hemanth Gantasala
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Sai Hemanth Gantasala
Gerrit-Comment-Date: Fri, 14 Jun 2024 08:59:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21478 to look at the new patch set (#2). Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. IMPALA-13120: Load failed table without need for manual invalidate If the metastore is down when the table load is triggered, catalogd updates a new version of incomplete table with cause as TableLoadingException. On coordinator/impalad, StmtMetadataLoader loadTables that has been waiting for table load to complete, considers the table as loaded. Then, during the analyzer’s table resolve step, for the incomplete table, TableLoadingException (i.e., Could not connect to meta store, failed to load metadata for table and running invalidate metadata for table may resolve this problem) is thrown. Henceforth, any query on the table doesn’t trigger the load since the table is incomplete with TableLoadingException cause. Even though metastore is UP later at some time, queries continue to throw the same exception. It is both misleading and also not required to manually invalidate all such tables. Note: Incomplete table with cause is considered as loaded. This patch tries again to load the table that has previously failed due to metastore connection error(i.e., a recoverable error), when a query involving the table is fired. Idea is to keep track of table object present in db that requires load. On successful/failed load, table object in db is updated. Therefore the tracked table object reference can be compared to table object in db to detect the completion of load. Testing: - Added end-to-end tests Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 --- M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M tests/custom_cluster/test_catalog_hms_failures.py 5 files changed, 86 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/21478/2 -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16346/ : 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/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 10:23:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 8: Code-Review+2 Thanks for the changes, lgtm! -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 11:29:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10712/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 11:40:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Hello Alexey Serbin, Riza Suminto, Ashwani Raina, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21492 to look at the new patch set (#4). Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. IMPALA-12370: allow converting timestamps to UTC when writing Kudu Before this commit only read support was implemented (convert_kudu_utc_timestamps=true). This change adds write support: if write_kudu_utc_timestamps=true, then timestamps are converted from local time to UTC during DMLs to Kudu. In case of ambigious conversions (DST changes) the earlier possible UTC timestamp is written. All DMLs supported with Kudu tables are affected affected: INSERT, UPSERT, UPDATE, DELETE To be able to read back Kudu tables written by Impala correctly convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to have the same value. Having the same value in the two query option is also critical for UPDATE/DELETE if the primary key contains a timestamp column - these operations do a scan first (affected by convert_kudu_utc_timestamps) and then use the keys from the scan to select updated/deleted rows (affected by write_kudu_utc_timestamps). The conversion is implemented by adding to_utc_timestamp() to inserted timestamp expressions during planning. This allows doing the same conversion during the pre-insert sorting and partitioning. Read support is implemented differently - in that case the plan is not changed and the scanner does the conversion. Other changes: - Before this change, verification of tests with TIMESTAMP results were skipped when the file format is Kudu. This shouldn't be necessary so the skipping was removed. Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/KuduModifyImpl.java M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java M fe/src/main/java/org/apache/impala/util/ExprUtil.java M testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test M tests/common/test_result_verifier.py M tests/query_test/test_kudu.py 13 files changed, 226 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21492/4 -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 4: PS4 doesn't resolve the latest comments but adds update/delete support and tests (I forgot this in the first run) -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 12:31:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: allow converting timestamps to UTC when writing Kudu
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: allow converting timestamps to UTC when writing Kudu .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16347/ : 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/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 12:53:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12732: Add support for MERGE statements for Iceberg tables
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/21423 )
Change subject: IMPALA-12732: Add support for MERGE statements for Iceberg
tables
..
Patch Set 7:
(22 comments)
doing another dump. Still not finished with the analyzer part, but getting
there soon.
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/DmlStatementBase.java
File fe/src/main/java/org/apache/impala/analysis/DmlStatementBase.java:
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/DmlStatementBase.java@92
PS7, Line 92: protected void checkSubQuery(SlotRef lhsSlotRef, Expr rhsExpr)
These functions, if I don't miss anything, can work without an actual object of
this class as they receive all information via params and don't need a state.
Can they be static to the class?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java
File fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java:
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@189
PS7, Line 189: (MergeCase)
Is it necessary to convert the items to MergeCase. I believe they are of
MergeCase type already.
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@225
PS7, Line 225: public Expr getRowPresentExpr() {
Haven't checked all of these but at least some of them are only called within
this class. They could be private then.
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@246
PS7, Line 246:* SELECT /* +straight_join */
nit: could you indent this example SQL for better readability?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@247
PS7, Line 247:* CAST(TupleIsNull(0) + TupleIsNull(1) * 2 AS TINYINT)
row_present,
this is rather:
CAST(TupleIsNull(0) + CAST(TupleIsNull(1) * 2 AS TINYINT)
1: pls update the comment
2: do we need the inner cast to TINYINT after we multiple a bool with a tinyint?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@262
PS7, Line 262: targetColumns
this is rather 'targetSlotRefs'
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@263
PS7, Line 263: targetTableRef_.getTable()
: .getColumns()
: .stream()
: .map(column
: -> new SlotRef(
:
ImmutableList.of(targetTableRef_.getUniqueAlias(),
: column.getName(
: .collect(Collectors.toList());
not sure if this is auto formatted, but I find it more compact and readable if
we indent like this:
tblRef.getTable.getColumns().stream()
.map()
.collect()
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@362
PS7, Line 362: sortColumns
this is rather sortColumnPositions or sortColumnIndexes, right?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@365
PS7, Line 365: resultExpressions::get
could you write expressions_.resultExpressions()::get (or similar) here and get
rid of L363?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@372
PS7, Line 372: in the same format
nit: what do you mean by 'in the same format'?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@374
PS7, Line 374: protected static class MergeExpressions {
If I'm not mistaken, this class is to store intermediate states of this class,
but this doesn't really add any extra logic on top. I feel that anything that
is within this class could be simply members of the outer class. And then we
would save all these getters/setters. No strong feelings, though.
If we decide to keep this, then I think the members should be public to get rid
of these getters/setters.
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@378
PS7, Line 378: private final List sortingOrder_;
this name is confusing with the one 2 lines below. What if we called this
sortingColumnPositions or such?
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@391
PS7, Line 391: private List targetExpressions;
nit: member names should have a '_' suffix
http://gerrit.cloudera.org:8080/#/c/21423/7/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java@399
PS7, Line 399: public MergeExpressi
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h@642 PS2, Line 642: uint64_t It seems safer to not change this as it is a public const. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:06:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10713/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 14:14:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10714/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 14:29:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder
Hello Gabor Kaszab, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21452 to look at the new patch set (#4). Change subject: IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder .. IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder With this patch IcebergDeleteBuilder checks how many probe threads are actually blocked on the builder. Let's assume the following plan: UNION ALL / \ / \ / \ SCAN allANTI JOIN datafiles / \ without / \ deletes SCAN SCAN datafilesdeletes with deletes In that case UNION ALL, and the two "SCAN datafiles" operators are in the same fragment, while the builder of the ANTI JOIN is in a different fragment. This means that "SCAN datafiles without deletes" can run in parallel with the builder. But once that SCAN is exhausted, the UNION ALL will drain rows from "SCAN datafiles with deletes" via the ANTI JOIN operator, but that operator depends on the join builder output. This means in some cases the SCAN fragments are busy, while in other cases the SCAN fragments are blocked. It depends on how much work they need to do, and how much work the build-side needs to do. So to handle all cases, we dynamically check how many probe fragments are blocked on the builder, then spin up as many threads to parellelize the final sort. This also works well when we have the following plan: ANTI JOIN / \ / \ SCAN SCAN datafilesdeletes with deletes The above plan is created when all data files have corresponding deletes, or when we are running a simple count(*) query. In that case all "SCAN datafiles" fragments are blocked on the builder, so we can use that many threads to sort the build results. A new field "ThreadCountInFinalBuild" was added, so we can check the query profile about how many threads were used for the final sorting in the builders. Measurements: In a table with 1 Trillion data records and 68.5 Billion delete records it reduced "IcebergDeletePositionSortTimer" from ~1 minute to 8-10 seconds, in an environment with 40 executors and MT_DOP=12. Testing: * e2e tests that check counter "ThreadCountInFinalBuild" Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 --- M be/src/exec/iceberg-delete-builder.cc M be/src/exec/iceberg-delete-builder.h M be/src/exec/join-builder.cc M be/src/exec/join-builder.h M testdata/workloads/functional-query/queries/QueryTest/iceberg-update-stress.test M tests/stress/test_update_stress.py 6 files changed, 159 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/21452/4 -- To view, visit http://gerrit.cloudera.org:8080/21452 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 Gerrit-Change-Number: 21452 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 89 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21501/3 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 9: Code-Review+2 Carry +2 from Csaba after rebase. -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 14:28:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h@642 PS2, Line 642: unsigned > It seems safer to not change this as it is a public const. Done -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:19:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Hello Yida Wu, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21455 to look at the new patch set (#9). Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB ExprValuesCache uses BATCH_SIZE as a deciding factor to set its capacity. It bounds the capacity such that expr_values_array_ memory usage stays below 256KB. This patch tightens that limit to include all memory usage from ExprValuesCache::MemUsage() instead of expr_values_array_ only. Therefore, setting a very high BATCH_SIZE will not push the total memory usage of ExprValuesCache beyond 256KB. Simplify table dimension creation methods and fix few flake8 warnings in test_dimensions.py. Testing: - Add test_join_queries.py::TestExprValueCache. - Pass core tests. Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 --- M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M bin/rat_exclude_files.txt A testdata/workloads/tpcds_partitioned/queries M tests/common/test_dimensions.py M tests/query_test/test_join_queries.py 6 files changed, 70 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/21455/9 -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16348/ : 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/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:44:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Surya Hebbar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. IMPALA-13137: Add additional client fetch metrics columns to the queries page For helping users to better understand query execution times, the following columns have been added to the queries page. - First row fetched - Client fetch wait time The duration needed to fetch the first row is already present in the query state record. This timestamp has to be found by matching the label "First row fetched" within the query state record's 'event_sequence'. The time taken for the client to fetch all rows since the first row is monitored by the "ClientFetchWaitTimer" in client request state. To incorporate this into the query state record, the following new attribute has been added. - client_fetch_wait_time_ns In order to constantly retrieve and update this value, a new method has been added to expose the private counter holding "ClientFetchWaitTimer". The duration for both these columns is printed in human readable format, i.e. seconds, minutes, hours, etc., after converting from nanoseconds. Tips for the two additional columns have also been added to the queries page. Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/service/client-request-state.h M be/src/service/impala-http-handler.cc M be/src/service/query-state-record.cc M be/src/service/query-state-record.h M www/queries.tmpl 7 files changed, 58 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21482/5 -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 5 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16350/ : 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/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 14:53:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21452 ) Change subject: IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16349/ : 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/21452 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 Gerrit-Change-Number: 21452 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 14:52:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG@11 PS4, Line 11: - First row fetched : - Client fetch wait time > Match this with chosen label in queries.tmpl Done http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG@18 PS4, Line 18: fetch all rows > "fetch all rows since the first row" Done http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc@463 PS4, Line 463: " fetch all rows s > " fetch all rows since the first row." Done http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc@548 PS4, Line 548: Coordinator::PROFIL > Make this a constant and share it with the same occurrence in coordinator.c Done http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl File www/queries.tmpl: http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl@117 PS4, Line 117: First row fetched : : Client fetch wait time > nit: Since this column is all about Client time, I'd suggest shorter label Yeah, I can agree with it. But, it may cause a misunderstanding, as the same values are named differently in the profile. These are accurate and good names for these columns. So, I will go ahead with "First Fetch" and "Fetch Duration" if confusion between profile is okay. -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 5 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 15:05:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/21482/5/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/21482/5/be/src/service/impala-http-handler.cc@548 PS5, Line 548: record.event_sequence.labels.end(), Coordinator::PROFILE_EVENT_LABEL_FIRST_ROW_FETCHED); line too long (94 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 5 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 15:05:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16351/ : 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/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 5 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 15:29:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21452 ) Change subject: IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10715/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21452 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 Gerrit-Change-Number: 21452 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 15:56:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG@18 PS4, Line 18: fetch all rows > Done Done http://gerrit.cloudera.org:8080/#/c/21482/4//COMMIT_MSG@18 PS4, Line 18: fetch all rows > Done Done http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc@463 PS4, Line 463: " fetch all rows s > Done Done http://gerrit.cloudera.org:8080/#/c/21482/4/be/src/service/impala-http-handler.cc@548 PS4, Line 548: Coordinator::PROFIL > Done Done http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl File www/queries.tmpl: http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl@117 PS4, Line 117: First row fetched : : Client fetch wait time > Yeah, I can agree with it. But, it may cause a misunderstanding, as the sam Different name between this table header and query profile is OK. I'd say go for it. You can clarify the association with query profile in the tooltip message. I think we should go with "First Fetch" and "Fetch Time". "Duration" is too long. http://gerrit.cloudera.org:8080/#/c/21482/5/www/queries.tmpl File www/queries.tmpl: http://gerrit.cloudera.org:8080/#/c/21482/5/www/queries.tmpl@54 PS5, Line 54: Queued Duration Please change this to "Queue Time" to save space. http://gerrit.cloudera.org:8080/#/c/21482/5/www/queries.tmpl@124 PS5, Line 124: Duration Please change this to "Queue Time" to save space. -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 5 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 15:57:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Hello Alexey Serbin, Riza Suminto, Ashwani Raina, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21492 to look at the new patch set (#5). Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu Before this commit, only read support was implemented (convert_kudu_utc_timestamps=true). This change adds write support: if write_kudu_utc_timestamps=true, then timestamps are converted from local time to UTC during DMLs to Kudu. In case of ambigious conversions (DST changes) the earlier possible UTC timestamp is written. All DMLs supported with Kudu tables are affected affected: INSERT, UPSERT, UPDATE, DELETE To be able to read back Kudu tables written by Impala correctly convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to have the same value. Having the same value in the two query option is also critical for UPDATE/DELETE if the primary key contains a timestamp column - these operations do a scan first (affected by convert_kudu_utc_timestamps) and then use the keys from the scan to select updated/deleted rows (affected by write_kudu_utc_timestamps). The conversion is implemented by adding to_utc_timestamp() to inserted timestamp expressions during planning. This allows doing the same conversion during the pre-insert sorting and partitioning. Read support is implemented differently - in that case the plan is not changed and the scanner does the conversion. Other changes: - Before this change, verification of tests with TIMESTAMP results were skipped when the file format is Kudu. This shouldn't be necessary so the skipping was removed. Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/KuduModifyImpl.java M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java M fe/src/main/java/org/apache/impala/util/ExprUtil.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv M testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test M tests/common/test_result_verifier.py M tests/query_test/test_kudu.py 16 files changed, 263 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21492/5 -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@7 PS3, Line 7: writing > nit: writing to Done http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@7 PS3, Line 7: Allow > nit: Allow Done http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@9 PS3, Line 9: commit > nit: commit, Done http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@16 PS3, Line 16: All DMLs supported with Kudu tables are affected affected: : INSERT, UPSERT, UPDATE, DELETE : > Maybe a PlannerTest to see that to_utc_timestamp / utc_to_unix_micros is ad Added planner tests. http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@16 PS3, Line 16: All DMLs supported with Kudu tables are affected affected: : INSERT, UPSERT, UPDATE, DELETE : > Add validation in impala::ValidateQueryOptions. I am not 100% sure about the correct way to handle the query options. If the two have different values, then UPDATE/DELETE can be completely wrong. Meanwhile this is an advanced undocumented feature, so we can assume that the user knows what they are doing. If this will be properly supported in the future, then I think that it would be better to allow setting it per table with a table property that overrides the query options. But I wouldn't do this in the current commit. http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java File fe/src/main/java/org/apache/impala/util/ExprUtil.java: http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java@126 PS3, Line 126: /** :* Wraps 'timestampExpr' in to_utc_timestamp() that converts > Add comment for this method. I think this wrap timestampExpr inside a Funct Done -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 16:05:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16352/ : 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/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 16:26:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Hello Alexey Serbin, Riza Suminto, Ashwani Raina, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21492 to look at the new patch set (#6). Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu Before this commit, only read support was implemented (convert_kudu_utc_timestamps=true). This change adds write support: if write_kudu_utc_timestamps=true, then timestamps are converted from local time to UTC during DMLs to Kudu. In case of ambigious conversions (DST changes) the earlier possible UTC timestamp is written. All DMLs supported with Kudu tables are affected affected: INSERT, UPSERT, UPDATE, DELETE To be able to read back Kudu tables written by Impala correctly convert_kudu_utc_timestamps and write_kudu_utc_timestamps need to have the same value. Having the same value in the two query option is also critical for UPDATE/DELETE if the primary key contains a timestamp column - these operations do a scan first (affected by convert_kudu_utc_timestamps) and then use the keys from the scan to select updated/deleted rows (affected by write_kudu_utc_timestamps). The conversion is implemented by adding to_utc_timestamp() to inserted timestamp expressions during planning. This allows doing the same conversion during the pre-insert sorting and partitioning. Read support is implemented differently - in that case the plan is not changed and the scanner does the conversion. Other changes: - Before this change, verification of tests with TIMESTAMP results were skipped when the file format is Kudu. This shouldn't be necessary so the skipping was removed. Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaService.thrift M common/thrift/Query.thrift M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/KuduModifyImpl.java M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java M fe/src/main/java/org/apache/impala/util/ExprUtil.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv A testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test M testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test M tests/common/test_result_verifier.py M tests/query_test/test_kudu.py 17 files changed, 379 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/21492/6 -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 6: PS6 rebased the patch (and resolved query option related conflicts) and added kudu-dml-with-utc-conversion.test which I forgot in the last commit -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 16:30:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13088: (part 1) Improve build batch processing of IcebergDeleteBuilder
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21435 ) Change subject: IMPALA-13088: (part 1) Improve build batch processing of IcebergDeleteBuilder .. Patch Set 3: (9 comments) http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.h File be/src/exec/iceberg-delete-builder.h: http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.h@70 PS3, Line 70: d Nit: this should be "Processes". http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.h@121 PS3, Line 121: The newly added vectors have double capacity so we don't end up having : // too many vectors Could you clarify this? Is it about 'intermediate_delete_rows_' in addition to the final map, or the embedded vectors? http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.h@149 PS3, Line 149: & We usually take non-const (output) arguments by pointer. http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc File be/src/exec/iceberg-delete-builder.cc: http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@184 PS3, Line 184: auto I think using the actual type (IntermediateDeleteRowVector&) is more readable. http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@242 PS3, Line 242: auto I think using the actual type (IntermediateDeleteRowVector&) is more readable. http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@250 PS3, Line 250: auto I think using the actual type (vector) is more readable. Also L252. http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@296 PS3, Line 296: auto I think using the actual type (vector) is more readable. Also L306. http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@319 PS3, Line 319: StringValue* Can we take it by const ref? http://gerrit.cloudera.org:8080/#/c/21435/3/be/src/exec/iceberg-delete-builder.cc@327 PS3, Line 327: } Can it happen that (*path) is not a key in 'intermediate_delete_rows_' but path->Len() != 0? If not, we could add DCHECK(false) in a following ELSE branch, with an explaining comment. -- To view, visit http://gerrit.cloudera.org:8080/21435 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14541a064a522d4780fb5f02636736259e79b9cf Gerrit-Change-Number: 21435 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 16:37:40 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-13158: Skip building Thrift for rust/dotnet
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21515 ) Change subject: IMPALA-13158: Skip building Thrift for rust/dotnet .. Patch Set 3: Verified+1 Code-Review+2 Carry +2. -- To view, visit http://gerrit.cloudera.org:8080/21515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac3b3ca9dffc45853dc22d5c34089b9b9f9e242d Gerrit-Change-Number: 21515 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 14 Jun 2024 16:47:39 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-13158: Skip building Thrift for rust/dotnet
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21515 ) Change subject: IMPALA-13158: Skip building Thrift for rust/dotnet .. Patch Set 3: and passed full toolchain build. -- To view, visit http://gerrit.cloudera.org:8080/21515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac3b3ca9dffc45853dc22d5c34089b9b9f9e242d Gerrit-Change-Number: 21515 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 14 Jun 2024 16:47:54 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-13158: Skip building Thrift for rust/dotnet
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21515 ) Change subject: IMPALA-13158: Skip building Thrift for rust/dotnet .. IMPALA-13158: Skip building Thrift for rust/dotnet Skips building Thrift on rust (with-rs) and .NET core (dotnet in Thrift 11, netstd in Thrift 16). Avoids errors if rust compiler is old or dotnet missing an SDK. Change-Id: Iac3b3ca9dffc45853dc22d5c34089b9b9f9e242d Reviewed-on: http://gerrit.cloudera.org:8080/21515 Reviewed-by: Michael Smith Tested-by: Michael Smith --- M source/thrift/build.sh 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: Michael Smith: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iac3b3ca9dffc45853dc22d5c34089b9b9f9e242d Gerrit-Change-Number: 21515 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10712/ -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 16:52:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16353/ : 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/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 16:55:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13137: Add additional client fetch metrics columns to the queries page
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/21482 ) Change subject: IMPALA-13137: Add additional client fetch metrics columns to the queries page .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl File www/queries.tmpl: http://gerrit.cloudera.org:8080/#/c/21482/4/www/queries.tmpl@117 PS4, Line 117: First row fetched : : Client fetch wait time > Different name between this table header and query profile is OK. I'd say g "Fetch Time" would be misleading. This actually the time after the rows have been fetched from the server and can include transport and client processing or even the client doing nothing. -- To view, visit http://gerrit.cloudera.org:8080/21482 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74a9393a7b38750de0c3f6230b6e5e048048c4b5 Gerrit-Change-Number: 21482 Gerrit-PatchSet: 4 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 17:14:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 6: (15 comments) http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21492/3//COMMIT_MSG@16 PS3, Line 16: All DMLs supported with Kudu tables are affected affected: : INSERT, UPSERT, UPDATE, DELETE : > Added planner tests. Ack. Is it worth to at least print WARNING in Planner.getExplainString() if those two option values are not equal? If yes, please add test to show such WARNING is displayed in query profile. http://gerrit.cloudera.org:8080/#/c/21492/6/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/21492/6/common/thrift/ImpalaService.thrift@883 PS6, Line 883: conversiyon nit: conversion http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java File fe/src/main/java/org/apache/impala/util/ExprUtil.java: http://gerrit.cloudera.org:8080/#/c/21492/3/fe/src/main/java/org/apache/impala/util/ExprUtil.java@126 PS3, Line 126: /** :* Wraps 'timestampExpr' in to_utc_timestamp() that converts > Done Done http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/main/java/org/apache/impala/util/ExprUtil.java File fe/src/main/java/org/apache/impala/util/ExprUtil.java: http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/main/java/org/apache/impala/util/ExprUtil.java@104 PS6, Line 104: Preconditions.checkArgument(timestampExpr.isConstant()); Is this Precondition and another one in L86 correct? I see some examples where utc_to_unix_micros() is not wrapping constant expression. workloads/functional-planner/queries/PlannerTest/bloom-filter-assignment.test:435:| runtime filters: RF000[bloom] <- c.timestamp_col, RF001[bloom] <- utc_to_unix_micros(c.timestamp_col) workloads/functional-planner/queries/PlannerTest/bloom-filter-assignment.test:506:| runtime filters: RF000[bloom] <- c.timestamp_col, RF001[bloom] <- utc_to_unix_micros(c.timestamp_col) workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test:72:row_regex: .*RF00.\[bloom\] <- utc_to_unix_micros\(to_utc_timestamp\(from_utc_timestamp\(t2.ts, 'Asia/Shanghai'\), 'Asia/Shanghai'\)\).* http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/21492/6/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@753 PS6, Line 753: kudu-dml-with-utc-conversion Do you mind adding 1 SELECT test where CONVERT_KUDU_UTC_TIMESTAMPS affect plan? I don't see 'utc_to_unix_micros' anywhere in test file. http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv File testdata/datasets/functional/schema_constraints.csv: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv@425 PS6, Line 425: # The table is used to test DST changes in timestamps, the timestamps in the table near : # DST changes in the 'America/Los_Angeles' time zone. Please add in comment: "In 2011 Los Angeles, DST starts at 2 a.m. on Sunday, March 13 (clocks were turned forward 1 hour) and fall back to standard time again at 2 a.m. on Sunday, November 6 (clocks were turned backward 1 hour)." https://www.timeanddate.com/time/change/usa?year=2011. http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-planner/queries/PlannerTest/kudu-dml-with-utc-conversion.test@75 PS6, Line 75: Is this whitespace a defect from getExplainString() ? http://gerrit.cloudera.org:8080/#/c/21492/5/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/5/testdata/workloads/functional-query/queries/QueryTest/kudu_predicate_with_timestamp_conversion.test@20 PS5, Line 20: INT question: This was BIGINT before and it does not break test when timestamp_at_dst_changes.id type is INT. Why is that? http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/workloads/functional-query/queries/QueryTest/kudu_timestamp_conversion.test@31 PS
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21478 to look at the new patch set (#3). Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. IMPALA-13120: Load failed table without need for manual invalidate If the metastore is down when the table load is triggered, catalogd updates a new version of incomplete table with cause as TableLoadingException. On coordinator/impalad, StmtMetadataLoader loadTables that has been waiting for table load to complete, considers the table as loaded. Then, during the analyzer’s table resolve step, for the incomplete table, TableLoadingException (i.e., Could not connect to meta store, failed to load metadata for table and running invalidate metadata for table may resolve this problem) is thrown. Henceforth, any query on the table doesn’t trigger the load since the table is incomplete with TableLoadingException cause. Even though metastore is UP later at some time, queries continue to throw the same exception. It is both misleading and also not required to manually invalidate all such tables. Note: Incomplete table with cause is considered as loaded. This patch tries again to load the table that has previously failed due to metastore connection error(i.e., a recoverable error), when a query involving the table is fired. Idea is to keep track of table object present in db that requires load. On successful/failed load, table object in db is updated. Therefore the tracked table object reference can be compared to table object in db to detect the completion of load. Testing: - Added end-to-end tests Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 --- M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/FeIncompleteTable.java M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java M tests/custom_cluster/test_catalog_hms_failures.py 5 files changed, 87 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/21478/3 -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10716/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 18:10:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG@12 PS1, Line 12: This patch adds code to handle inconsistent : cluster membership state after statestore failover in the same way. Please clarify what is the expected behavior. When is query should cancel and not cancel? Is both failover scenario and regular StateStore restart scenario should not cancel RUNNING query? http://gerrit.cloudera.org:8080/#/c/21520/1/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/21520/1/be/src/statestore/statestore-subscriber.cc@1095 PS1, Line 1095: bool has_failed_before = connection_failure_metric_->GetValue() > 0; : bool in_failed_grace_period = MilliSecondsSinceLastRegistration() : < FLAGS_statestore_subscriber_recovery_grace_period_ms; I think 'has_disconnect_before' and 'in_disconnect_grace_period' is a better name to distinguish from failover scenario. My understanding is that, disconnect only means temporary network issue to StateStore, and is not changing from active StateStore to standby StateStore. http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@677 PS1, Line 677: # Wait for long enough for the standby statestored to detect the failure of active : # statestored and assign itself in active role. Is this 2s (statestore_peer_timeout_seconds)? http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. Why not let query finish? I thought this test want to show that query completes and not impacted by StateStore failover. -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 14 Jun 2024 18:12:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16354/ : 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/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 18:27:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 9: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10714/ -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 19:06:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21492 ) Change subject: IMPALA-12370: Allow converting timestamps to UTC when writing to Kudu .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv File testdata/datasets/functional/schema_constraints.csv: http://gerrit.cloudera.org:8080/#/c/21492/6/testdata/datasets/functional/schema_constraints.csv@425 PS6, Line 425: # The table is used to test DST changes in timestamps, the timestamps in the table near : # DST changes in the 'America/Los_Angeles' time zone. > Please add in comment: It is also good note to mention that during DST, PDT = UTC-7. While outside DST, PST = UTC-8. -- To view, visit http://gerrit.cloudera.org:8080/21492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4995a64e042e7bb261fcc6e6bf7ffce61e9bd1 Gerrit-Change-Number: 21492 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 14 Jun 2024 19:11:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/21455/9/tests/common/test_dimensions.py File tests/common/test_dimensions.py: http://gerrit.cloudera.org:8080/#/c/21455/9/tests/common/test_dimensions.py@127 PS9, Line 127: create_table_format_dimension(workload, 'text/none') Forgot to return here and below. -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 19:14:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Hello Yida Wu, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21455 to look at the new patch set (#10). Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB ExprValuesCache uses BATCH_SIZE as a deciding factor to set its capacity. It bounds the capacity such that expr_values_array_ memory usage stays below 256KB. This patch tightens that limit to include all memory usage from ExprValuesCache::MemUsage() instead of expr_values_array_ only. Therefore, setting a very high BATCH_SIZE will not push the total memory usage of ExprValuesCache beyond 256KB. Simplify table dimension creation methods and fix few flake8 warnings in test_dimensions.py. Testing: - Add test_join_queries.py::TestExprValueCache. - Pass core tests. Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 --- M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M bin/rat_exclude_files.txt A testdata/workloads/tpcds_partitioned/queries M tests/common/test_dimensions.py M tests/query_test/test_join_queries.py 6 files changed, 70 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/21455/10 -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 10: Code-Review+2 (1 comment) Carry +2 for trivial changes. http://gerrit.cloudera.org:8080/#/c/21455/9/tests/common/test_dimensions.py File tests/common/test_dimensions.py: http://gerrit.cloudera.org:8080/#/c/21455/9/tests/common/test_dimensions.py@127 PS9, Line 127: return create_table_format_dimension(workload, 'text > Forgot to return here and below. Done -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 19:20:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10717/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 19:21:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16355/ : 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/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 19:40:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: > Patch Set 16: Code-Review+1 > > Reviewed the FE changes. The patch looks harmless to me. Thanks for confirming this Quanlong! Does anyone else still have some concern or would like to change something? I think this is a rather safe change. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 14 Jun 2024 19:56:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. IMPALA-13159: Fix query cancellation caused by statestore failover A momentary inconsistent cluster membership state after statestore failover results in query cancellation. We already have code to handle inconsistent cluster membership after statestore restarting by defining a post-recovery grace period. During the grace period, don't update the current cluster membership so that the inconsistent membership will not be used to cancel queries on coordinators and executors. This patch handles inconsistent cluster membership state after statestore failover in the same way. Testing: - Added a new test case to verify that inconsistent cluster membership after statestore failover will not result in query cancellation. - Fixed closing client issue for Catalogd HA test case test_catalogd_failover_with_sync_ddl when the test fails. - Passed core test. Change-Id: I720bec5199df46475b954558abb0637ca7e6298b --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M tests/custom_cluster/test_catalogd_ha.py M tests/custom_cluster/test_statestored_ha.py 5 files changed, 112 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/21520/2 -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > If the query is cancelled, its state will be changed to QueryState.CANCELLE Finishing this query takes about 120 seconds on my local machine. It seems too long for one test case. Without waiting to finish, it takes about 19 seconds. -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 20:25:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16356/ : 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/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 20:45:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG@12 PS1, Line 12: y defining a post-recovery grace period. During : the grace period, don't update the current cluster membership so th > Define a post-recovery grace period after statestore has been disconnected Thanks! Follow up question: after grace period is over, is it the case that StateStore will send new cluster membership update to Coordinator and Coordinator will cancel the query because cluster membership has changed since query start RUNNING? http://gerrit.cloudera.org:8080/#/c/21520/1/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/21520/1/be/src/statestore/statestore-subscriber.cc@1095 PS1, Line 1095: bool has_disconnect_before = connection_failure_metric_->GetValue() > 0; : bool in_disconnect_grace_period = MilliSecondsSinceLastRegistration() : < FLAGS_statestore_subscriber_recovery_grace_period_ms; > Renamed two variables. Done http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@653 PS1, Line 653: """Test that a momentary inconsistent cluster membership state after statestore : service fail-over will not result in query cancellation. Also make sure that query : get cancelled if a backend actually went down.""" Is there an existing opposite test that shows query cancelled after grace period due to cluster membership difference? http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > Finishing this query takes about 120 seconds on my local machine. It seems In that case, is it better to cancel through query handle instead? client.cancel(handle) In this test, is the cancelation reason because cluster membership change due to stopped backend, or just because row transmission broken? -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 20:58:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21452 ) Change subject: IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10715/ -- To view, visit http://gerrit.cloudera.org:8080/21452 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 Gerrit-Change-Number: 21452 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 14 Jun 2024 21:08:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h File be/src/statestore/statestore-subscriber.h: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h@345 PS2, Line 345: return MonotonicMillis() - last_failover_time_.Load(); nit: Might be nice to DCHECK that the result is not negative. http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc@1098 PS2, Line 1098: bool has_failover_before = last_failover_time_.Load() > 0; Do we really need to check this? If it's 0, then MilliSecondsSinceLastFailover will be a very large number and not less than statestore_subscriber_recovery_grace_period_ms. Right now it means we read from an atomic twice in a row. http://gerrit.cloudera.org:8080/#/c/21520/2/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/2/tests/custom_cluster/test_statestored_ha.py@692 PS2, Line 692: assert False, "Query expected to fail" Can we verify that it had to wait longer than the CANCELLATION_GRACE_PERIOD_S for the query to fail? -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 21:25:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13156: Investigation: Set explicit credential provider for S3 builds
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21510 ) Change subject: IMPALA-13156: Investigation: Set explicit credential provider for S3 builds .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8c09f8d042a69c5d3227398c720ea38e1c7e12f Gerrit-Change-Number: 21510 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 14 Jun 2024 22:23:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13152: Avoid NaN, infinite, and negative ProcessingCost
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21504 ) Change subject: IMPALA-13152: Avoid NaN, infinite, and negative ProcessingCost .. Patch Set 3: Code-Review+1 (3 comments) General question, don't need to be addressed in this patch. Should we distinguish between missing cardinality information and 0 cardinality? 0 cardinality would mean no data to process while missing cardinality information means we don't have cardinality information likely due to missing stats and so we could probably use some default cardinality. http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@a2231 PS3, Line 2231: Why is this check no longer required? http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java@a399 PS3, Line 399: Why this check is no longer required? http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java@a420 PS3, Line 420: Same question. -- To view, visit http://gerrit.cloudera.org:8080/21504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib49c7ae397dadcb2cb69fde1850d442d33cdf177 Gerrit-Change-Number: 21504 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 22:38:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > In that case, is it better to cancel through query handle instead? What will happen if, after client.execute_async(slow_query), you start 4th Impalad, and then kill that 4th Impalad at this point? Will that stop the query, even if the 4th Impalad is not part of query execution? -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 22:42:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13152: Avoid NaN, infinite, and negative ProcessingCost
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21504 ) Change subject: IMPALA-13152: Avoid NaN, infinite, and negative ProcessingCost .. Patch Set 3: (3 comments) > Should we distinguish between missing cardinality information and 0 > cardinality? Agree, but I rather address it later in separate patch. The way it is now in this patch set 3, Planner will bias towards assigning to smaller executor group set if cardinality is unknown (ProcessingCost is likely to be 0 for PlanNode with unknown cardinality). http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@a2231 PS3, Line 2231: > Why is this check no longer required? This is removed to allow ProcessingCost computation if COMPUTE_PROCESSING_COST=False, but TEST_REPLAN=True or RuntimeEnv.INSTANCE.isTestEnv() is True. http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java@a399 PS3, Line 399: > Why this check is no longer required? This is removed to allow ProcessingCost computation if COMPUTE_PROCESSING_COST=False, but TEST_REPLAN=True or RuntimeEnv.INSTANCE.isTestEnv() is True. http://gerrit.cloudera.org:8080/#/c/21504/3/fe/src/main/java/org/apache/impala/planner/ScanNode.java@a420 PS3, Line 420: > Same question. This is removed to allow ProcessingCost computation if COMPUTE_PROCESSING_COST=False, but TEST_REPLAN=True or RuntimeEnv.INSTANCE.isTestEnv() is True. -- To view, visit http://gerrit.cloudera.org:8080/21504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib49c7ae397dadcb2cb69fde1850d442d33cdf177 Gerrit-Change-Number: 21504 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 22:49:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13156: Investigation: Set explicit credential provider for S3 builds
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21510 ) Change subject: IMPALA-13156: Investigation: Set explicit credential provider for S3 builds .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21510/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py File testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py: http://gerrit.cloudera.org:8080/#/c/21510/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py@152 PS1, Line 152: https://github.com/apache/impala/blob/master/bin/impala-config.sh#L668-L689 Using a permalink is better since the line highlights won't drift as the code changes: https://github.com/apache/impala/blob/9f5fbcd841a87b144f3410891fe6bf1f8fe4288c/bin/impala-config.sh#L668-L689 -- To view, visit http://gerrit.cloudera.org:8080/21510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8c09f8d042a69c5d3227398c720ea38e1c7e12f Gerrit-Change-Number: 21510 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 14 Jun 2024 23:00:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10716/ -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 14 Jun 2024 23:17:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13156: Investigation: Set explicit credential provider for S3 builds
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21510 ) Change subject: IMPALA-13156: Investigation: Set explicit credential provider for S3 builds .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21510/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py File testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py: http://gerrit.cloudera.org:8080/#/c/21510/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py@152 PS1, Line 152: https://github.com/apache/impala/blob/master/bin/impala-config.sh#L668-L689 > Using a permalink is better since the line highlights won't drift as the co I like using a release tag for a shorter URL. -- To view, visit http://gerrit.cloudera.org:8080/21510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia8c09f8d042a69c5d3227398c720ea38e1c7e12f Gerrit-Change-Number: 21510 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 14 Jun 2024 23:21:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. IMPALA-13159: Fix query cancellation caused by statestore failover A momentary inconsistent cluster membership state after statestore failover results in query cancellation. We already have code to handle inconsistent cluster membership after statestore restarting by defining a post-recovery grace period. During the grace period, don't update the current cluster membership so that the inconsistent membership will not be used to cancel queries on coordinators and executors. This patch handles inconsistent cluster membership state after statestore failover in the same way. Testing: - Added a new test case to verify that inconsistent cluster membership after statestore failover will not result in query cancellation. - Fixed closing client issue for Catalogd HA test case test_catalogd_failover_with_sync_ddl when the test fails. - Passed core test. Change-Id: I720bec5199df46475b954558abb0637ca7e6298b --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M tests/custom_cluster/test_catalogd_ha.py M tests/custom_cluster/test_statestored_ha.py 5 files changed, 113 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/21520/3 -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21520/1//COMMIT_MSG@12 PS1, Line 12: y defining a post-recovery grace period. During : the grace period, don't update the current cluster membership so th > Thanks! Active statestored keeps sending cluster membership periodically to all subscribers, including coordinators/executors. Sending intervals are 100 ms by default. After grace period, the collected cluster membership should be consistent and it's safe to use the current cluster membership to detect if a node is unhealthy and cancel running queries which are assigned to unhealthy nodes. http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h File be/src/statestore/statestore-subscriber.h: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h@345 PS2, Line 345: int64_t time_ms = MonotonicMillis() - last_failover_time_.Load(); > nit: Might be nice to DCHECK that the result is not negative. Added DCHECK http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc@1098 PS2, Line 1098: bool in_failover_grace_period = MilliSecondsSinceLastFailover() > Do we really need to check this? If it's 0, then MilliSecondsSinceLastFailo Right, removed has_failover_before http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@653 PS1, Line 653: """Test that a momentary inconsistent cluster membership state after statestore : service fail-over will not result in query cancellation. Also make sure that query : get cancelled if a backend actually went down aft > Is there an existing opposite test that shows query cancelled after grace p Second part of this test case shows query cancelled after grace period due to cluster membership change. Updated comments. http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > In that case, is it better to cancel through query handle instead? Cancellation reason is cluster membership change after grace period. http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > What will happen if, after client.execute_async(slow_query), you start 4th The query will not be affected if 4th impalad has no scheduled task. http://gerrit.cloudera.org:8080/#/c/21520/2/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/2/tests/custom_cluster/test_statestored_ha.py@692 PS2, Line 692: assert False, "Query expected to fail" > Can we verify that it had to wait longer than the CANCELLATION_GRACE_PERIOD Yes, need to add another test case. -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 23:51:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h File be/src/statestore/statestore-subscriber.h: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.h@345 PS2, Line 345: int64_t time_ms = MonotonicMillis() - last_failover_time_.Load(); > nit: Might be nice to DCHECK that the result is not negative. Done http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/21520/2/be/src/statestore/statestore-subscriber.cc@1098 PS2, Line 1098: bool in_failover_grace_period = MilliSecondsSinceLastFailover() > Do we really need to check this? If it's 0, then MilliSecondsSinceLastFailo Done -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 14 Jun 2024 23:51:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16357/ : 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/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Sat, 15 Jun 2024 00:16:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB ExprValuesCache uses BATCH_SIZE as a deciding factor to set its capacity. It bounds the capacity such that expr_values_array_ memory usage stays below 256KB. This patch tightens that limit to include all memory usage from ExprValuesCache::MemUsage() instead of expr_values_array_ only. Therefore, setting a very high BATCH_SIZE will not push the total memory usage of ExprValuesCache beyond 256KB. Simplify table dimension creation methods and fix few flake8 warnings in test_dimensions.py. Testing: - Add test_join_queries.py::TestExprValueCache. - Pass core tests. Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Reviewed-on: http://gerrit.cloudera.org:8080/21455 Reviewed-by: Riza Suminto Tested-by: Impala Public Jenkins --- M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M bin/rat_exclude_files.txt A testdata/workloads/tpcds_partitioned/queries M tests/common/test_dimensions.py M tests/query_test/test_join_queries.py 6 files changed, 70 insertions(+), 22 deletions(-) Approvals: Riza Suminto: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21455 ) Change subject: IMPALA-13075: Cap memory usage for ExprValuesCache at 256KB .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee27cbbe8d3100301d05a6516b62c45975a8d0e0 Gerrit-Change-Number: 21455 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 15 Jun 2024 00:28:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13159: Fix query cancellation caused by statestore failover
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21520 ) Change subject: IMPALA-13159: Fix query cancellation caused by statestore failover .. Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py File tests/custom_cluster/test_statestored_ha.py: http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@653 PS1, Line 653: """Test that a momentary inconsistent cluster membership state after statestore : service fail-over will not result in query cancellation. Also make sure that query : get cancelled if a backend actually went down aft > Second part of this test case shows query cancelled after grace period due Ack http://gerrit.cloudera.org:8080/#/c/21520/1/tests/custom_cluster/test_statestored_ha.py@688 PS1, Line 688: # Now kill a backend, and make sure the query fails. > The query will not be affected if 4th impalad has no scheduled task. Ack -- To view, visit http://gerrit.cloudera.org:8080/21520 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I720bec5199df46475b954558abb0637ca7e6298b Gerrit-Change-Number: 21520 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Sat, 15 Jun 2024 01:32:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13120: Load failed table without need for manual invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21478 ) Change subject: IMPALA-13120: Load failed table without need for manual invalidate .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10718/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia882fdd865ef716351be7f1eaf203a9fb04c1c15 Gerrit-Change-Number: 21478 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Sat, 15 Jun 2024 05:14:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 17: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 17 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 15 Jun 2024 05:15:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 17: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10719/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 17 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 15 Jun 2024 05:15:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21160 ) Change subject: IMPALA-12921, IMPALA-12985: Support running Impala with locally built Ranger .. Patch Set 16: Code-Review+2 Merging this. Thank Fang-Yu! -- To view, visit http://gerrit.cloudera.org:8080/21160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I268d6d4d6e371da7497aac8d12f78178d57c6f27 Gerrit-Change-Number: 21160 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 15 Jun 2024 05:15:07 + Gerrit-HasComments: No
