[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16042/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Sat, 27 Apr 2024 04:56:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 10: (2 comments) > Patch Set 10: > > Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10587/ > DRY_RUN=true http://gerrit.cloudera.org:8080/#/c/21131/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21131/9//COMMIT_MSG@18 PS9, Line 18: '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F', > I don't see '\x7F' in the final code, did you change the approach and not u Done http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@69 PS6, Line 69: *out = ""; > Not without an adapter. string_view(in, in_len) would work. Done -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Sat, 27 Apr 2024 04:37:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10587/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Sat, 27 Apr 2024 04:32:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters The error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte was matching with one of the two bytes that represented the '\u00FF' literal. A set containing all the special characters has been included and, '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. Testing: Some basic tests for both parquet and iceberg are provided in unicode-column-name.test. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 2 files changed, 79 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/10 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[native-toolchain-CR] IMPALA-13020: Use 64-bit integer for Thrift max message size on C++
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21343 ) Change subject: IMPALA-13020: Use 64-bit integer for Thrift max message size on C++ .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94fdd7b07fcc8dca0639839b40a9eff815090835 Gerrit-Change-Number: 21343 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 27 Apr 2024 03:59:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 13: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Sat, 27 Apr 2024 03:19:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that lowers 'query_log_write_interval_s' or 'query_log_max_queued'. Lowers grace period on test_query_log_table_flush_on_shutdown because ShutdownWorkloadManagement() is not started until the grace period ends. Updates "Adding/Removing local backend" to only apply to executors. It was only added for executors, but would be removed on dedicated coordinators as well (resulting in a DFATAL message during graceful shutdown). Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Reviewed-on: http://gerrit.cloudera.org:8080/21345 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/scheduling/cluster-membership-mgr.cc M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 3 files changed, 21 insertions(+), 14 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[native-toolchain-CR] IMPALA-13020: Use 64-bit integer for Thrift max message size on C++
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21343 ) Change subject: IMPALA-13020: Use 64-bit integer for Thrift max message size on C++ .. Patch Set 2: (2 comments) Updated Thrift change: https://github.com/joemcdonnell/thrift/commit/7eaca019499ec8d9e1e739170197c33b1d6eb8e7 http://gerrit.cloudera.org:8080/#/c/21343/2/source/thrift/thrift-0.16.0-patches/0007-IMPALA-13020-Use-64-bit-integer-for-max-message-size.patch File source/thrift/thrift-0.16.0-patches/0007-IMPALA-13020-Use-64-bit-integer-for-max-message-size.patch: http://gerrit.cloudera.org:8080/#/c/21343/2/source/thrift/thrift-0.16.0-patches/0007-IMPALA-13020-Use-64-bit-integer-for-max-message-size.patch@35 PS2, Line 35: int maxFrameSize > Do we need to change this as well? This is only used by the TFramedTransport, which we don't use. Also, TFramedTransport has always had a limit on frame size, even in older Thrift versions like 0.11, and they just centralized it in TConfiguration. http://gerrit.cloudera.org:8080/#/c/21343/2/source/thrift/thrift-0.16.0-patches/0007-IMPALA-13020-Use-64-bit-integer-for-max-message-size.patch@79 PS2, Line 79: resetConsumedMessageSize > Do we need to change the parameter type of this too? Good catch, yes, we need this to use int64_t. Fixed. -- To view, visit http://gerrit.cloudera.org:8080/21343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94fdd7b07fcc8dca0639839b40a9eff815090835 Gerrit-Change-Number: 21343 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 27 Apr 2024 01:20:18 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-13020: Use 64-bit integer for Thrift max message size on C++
Hello Quanlong Huang, Csaba Ringhofer, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21343 to look at the new patch set (#3). Change subject: IMPALA-13020: Use 64-bit integer for Thrift max message size on C++ .. IMPALA-13020: Use 64-bit integer for Thrift max message size on C++ Currently, Thrift's max message size is specified with a 32-bit signed integer, so it maxes out at 2GB. Impala has use cases that can produce messages larger than 2GB, so this patches Thrift to change the max message size to be an int64_t. This will allow Impala to specify a limit larger than 2GB. This only applies to Thrift's C++ code and it does not change Java. Change-Id: I94fdd7b07fcc8dca0639839b40a9eff815090835 --- M buildall.sh A source/thrift/thrift-0.16.0-patches/0007-IMPALA-13020-Use-64-bit-integer-for-max-message-size.patch 2 files changed, 133 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/43/21343/3 -- To view, visit http://gerrit.cloudera.org:8080/21343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I94fdd7b07fcc8dca0639839b40a9eff815090835 Gerrit-Change-Number: 21343 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. IMPALA-13012: Lower default query_log_max_queued Sets the query_log_max_queued default such that query_log_max_queued * num_columns(49) < statement_expression_limit to avoid triggering e.g. AnalysisException: Exceeded the statement expression limit (25) Statement has 370039 expressions. Also increases statement_expression_limit for insertion to avoid an error if query_log_max_queued is changed. Logs time taken to write to the queries table for help with debugging and adds histogram "impala-server.completed-queries.write-durations". Fixes InternalServer so it uses 'default_query_options'. Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Reviewed-on: http://gerrit.cloudera.org:8080/21351 Reviewed-by: Michael Smith Tested-by: Michael Smith Reviewed-by: Riza Suminto --- M be/src/service/impala-server.h M be/src/service/internal-server-test.cc M be/src/service/internal-server.cc M be/src/service/internal-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/service/workload-management-flags.cc M be/src/service/workload-management.cc M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/SystemTables.thrift M common/thrift/metrics.json M tests/custom_cluster/test_query_log.py 13 files changed, 115 insertions(+), 71 deletions(-) Approvals: Michael Smith: Looks good to me, but someone else must approve; Verified Riza Suminto: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. Patch Set 8: Code-Review+2 Did another pass. Looks OK to go. Changed my vote to +2. -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 23:45:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. Patch Set 8: Verified+1 Code-Review+1 Carry +1s after merging parent. -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 23:24:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 23:24:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. IMPALA-13005: Create Query Live table in HMS Creates the 'sys.impala_query_live' table in HMS using a similar 'CREATE TABLE' command to 'sys.impala_query_log'. Updates frontend to identify a System Table based on the '__IMPALA_SYSTEM_TABLE' property. Tables improperly marked with '__IMPALA_SYSTEM_TABLE' will error when attempting to scan them because no relevant scanner will be available. Creating the table in HMS simplifies supporting 'SHOW CREATE TABLE' and 'DESCRIBE EXTENDED', so allows them for parity with Query Log. Explicitly disables 'COMPUTE STATS' on system tables as it doesn't work correctly. Makes System Tables work with local catalog mode, fixing LocalCatalogException: Unknown table type for table sys.impala_query_live Updates workload management implementation to rely more on SystemTables.thrift definition, and adds DCHECKs to verify completeness and ordering. Testing: - adds additional test cases for changes to introspection commands - passes existing test_query_live and test_query_log suites Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Reviewed-on: http://gerrit.cloudera.org:8080/21302 Reviewed-by: Riza Suminto Tested-by: Impala Public Jenkins --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/exec/system-table-scanner.cc M be/src/service/workload-management-fields.cc M be/src/service/workload-management.cc M be/src/service/workload-management.h M common/thrift/CatalogObjects.thrift M common/thrift/SystemTables.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java A fe/src/main/java/org/apache/impala/analysis/SystemTableRef.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java A fe/src/main/java/org/apache/impala/catalog/FeSystemTable.java M fe/src/main/java/org/apache/impala/catalog/SystemTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java A fe/src/main/java/org/apache/impala/catalog/local/LocalSystemTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/catalog/SystemTableTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java D testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test M tests/custom_cluster/test_query_live.py 26 files changed, 435 insertions(+), 294 deletions(-) Approvals: Riza Suminto: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 16 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 23:17:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21357 )
Change subject: IMPALA-12935: First pass on Calcite planner functions
..
Patch Set 2:
(2 comments)
Looks like this stack needs a rebase. I can't pull it locally either, I get an
"internal server error".
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@119
PS2, Line 119: public static List
createRelDataTypesForArgs(List impalaTypes) {
How does this differ from createRelDataTypes at line 318?
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@130
PS2, Line 130: public static RelDataType getRelDataType(Type impalaType) {
I'm not sure why this and createRelDataType both exist. They're very similar
except this one doesn't handle Decimal types.
--
To view, visit http://gerrit.cloudera.org:8080/21357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88
Gerrit-Change-Number: 21357
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:37:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16041/ : 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/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 22:27:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21194 ) Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21194/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21194/5//COMMIT_MSG@14 PS5, Line 14: on top of the Parser.jj file or the config.fmpp file in the futre are Impala typo: future -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 26 Apr 2024 22:26:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10586/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 22:23:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 22:23:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16040/ : 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/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 22:16:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 12: Code-Review+2 Looks good. Thanks for digging into it! -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 22:12:19 + Gerrit-HasComments: No
[Impala-ASF-CR] Refactor Workload Management
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: Refactor Workload Management .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21358/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21358/1//COMMIT_MSG@6 PS1, Line 6: : Refactor Workload Management > Please assign JIRA number. Will do, this refactor isn't urgent but I wanted to toss up some ideas. http://gerrit.cloudera.org:8080/#/c/21358/1/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/21358/1/tests/custom_cluster/test_query_log.py@255 PS1, Line 255: "--shutdown_grace_period_s=10 " TODO: when 'impalad_graceful_shutdown=True', add reasonable default 'shutdown_grace_period_s' and 'shutdown_deadline_s'. 'shutdown_grace_period_s=0' is probably fine, as test queries are fast and ShutdownWorkloadManagement doesn't run until after the grace period. -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 22:07:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21345 )
Change subject: IMPALA-12997: Use graceful shutdown for query log tests
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399
PS10, Line 399: if (local_be_desc->is_quiescing()) {
> Oh actually this is way more straight-forward. The next clause is "else if
Done
--
To view, visit http://gerrit.cloudera.org:8080/21345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:03:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21345 to look at the new patch set (#12). Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that lowers 'query_log_write_interval_s' or 'query_log_max_queued'. Lowers grace period on test_query_log_table_flush_on_shutdown because ShutdownWorkloadManagement() is not started until the grace period ends. Updates "Adding/Removing local backend" to only apply to executors. It was only added for executors, but would be removed on dedicated coordinators as well (resulting in a DFATAL message during graceful shutdown). Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc --- M be/src/scheduling/cluster-membership-mgr.cc M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 3 files changed, 21 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/12 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399 PS10, Line 399: VLOG(1) << "Removing local backend from group " << group.DebugString > Yeah, I'll update this to only pass false if it's a dedicated coordinator. Oh actually this is way more straight-forward. The next clause is "else if is_executor", so only executors add local backends. I'm going to clean this up. -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:58:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21345 to look at the new patch set (#11). Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that lowers 'query_log_write_interval_s' or 'query_log_max_queued'. Lowers grace period on test_query_log_table_flush_on_shutdown because ShutdownWorkloadManagement() is not started until the grace period ends. Skips logging DFATAL if the local backend executor has not been added to an executor group, as happens on dedicated coordinators. Avoids Tried to remove non-existing backend from per-host list: 127.0.0.1:... and a crash with minidump when doing graceful shutdown on dedicated coordinators. Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/executor-group.cc M be/src/scheduling/executor-group.h M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 5 files changed, 29 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/11 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399 PS10, Line 399: // Local backend may not have been added in a dedicated coordinator. > What is "Local backend" here? Different impalad in the same host, or This i Yeah, I'll update this to only pass false if it's a dedicated coordinator. "Local backend" means this impalad. I'm not entirely sure why it 'local_be_desc->executor_groups()' becomes non-empty on dedicated coordinators; it may be that during tests it doesn't have long enough to register a membership update after executor groups are added. -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:32:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399 PS10, Line 399: // Local backend may not have been added in a dedicated coordinator. What is "Local backend" here? Different impalad in the same host, or This impalad when it is not a dedicated coordinator? Can we add DCHECK to confirm that this is indeed an impalad configured as dedicated coordinator, or set expect_exists param in RemoveExecutorAndGroup to false if and only if this is a dedicated coordinator? -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:29:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 5: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/21326/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21326/5//COMMIT_MSG@14 PS5, Line 14: When catalogd collects the next : catalog update, they will be collected. HdfsTable then clears the set. nit: can you make it more clear? http://gerrit.cloudera.org:8080/#/c/21326/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java: http://gerrit.cloudera.org:8080/#/c/21326/5/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@30 PS5, Line 30: import java.util.stream.Collectors; nit: unused import -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Fri, 26 Apr 2024 21:26:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13039: AES Encryption/ Decryption Support in Impala
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20447 ) Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/exprs/string-functions-ir.cc@1863 PS9, Line 1863: EncryptionKey *t = new EncryptionKey(); Why use new for a local object? Just instantiate it EncryptionKey t; http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/exprs/string-functions-ir.cc@1901 PS9, Line 1901: delete t; > Isn't this a memory leak if the code returns at line 1894? Yes, should definitely use unique_ptrs. -- To view, visit http://gerrit.cloudera.org:8080/20447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 Gerrit-Change-Number: 20447 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 26 Apr 2024 21:17:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Impala Public Jenkins has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/21340 )
Change subject: IMPALA-13024: Ignore slots if using default pool and empty group
..
IMPALA-13024: Ignore slots if using default pool and empty group
Slot based admission should not be enabled when using default pool.
There is a bug where coordinator-only query still does slot based
admission because executor group name set to
ClusterMembershipMgr::EMPTY_GROUP_NAME ("empty group (using coordinator
only)"). This patch adds check to recognize coordinator-only query in
default pool and skip slot based admission for it.
Testing:
- Add BE test AdmissionControllerTest.CanAdmitRequestSlotsDefault.
- In test_executor_groups.py, split test_coordinator_concurrency to
test_coordinator_concurrency_default and
test_coordinator_concurrency_two_exec_group_cluster to show the
behavior change.
- Pass core tests in ASAN build.
Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0
Reviewed-on: http://gerrit.cloudera.org:8080/21340
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M tests/common/impala_connection.py
M tests/custom_cluster/test_executor_groups.py
9 files changed, 128 insertions(+), 18 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0
Gerrit-Change-Number: 21340
Gerrit-PatchSet: 10
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:18:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16039/ : 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/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:13:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21131/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21131/9//COMMIT_MSG@18
PS9, Line 18: '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F',
I don't see '\x7F' in the final code, did you change the approach and not
update the commit? Daniel's comment suggested we should include it.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@69
PS6, Line 69: if (in.empty()) {
> Could use a range-based for-loop: for (char ch : in) {...}.
Not without an adapter. string_view(in, in_len) would work.
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:10:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16038/ : 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/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 21:02:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21345 to look at the new patch set (#10). Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that lowers 'query_log_write_interval_s' or 'query_log_max_queued'. Lowers grace period on test_query_log_table_flush_on_shutdown because ShutdownWorkloadManagement() is not started until the grace period ends. Skips logging DFATAL if the local backend executor has not been added to an executor group, as happens on dedicated coordinators. Avoids Tried to remove non-existing backend from per-host list: 127.0.0.1:... and a crash with minidump when doing graceful shutdown on dedicated coordinators. Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/executor-group.cc M be/src/scheduling/executor-group.h M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 5 files changed, 27 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/10 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21345 to look at the new patch set (#9). Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that sets 'query_log_write_interval_s' or 'query_log_max_queued'. Skips logging DFATAL if the local backend executor has not been added to an executor group, as happens on dedicated coordinators. Avoids Tried to remove non-existing backend from per-host list: 127.0.0.1:... and a crash with minidump when doing graceful shutdown on dedicated coordinators. Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/executor-group.cc M be/src/scheduling/executor-group.h M tests/common/custom_cluster_test_suite.py M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 6 files changed, 38 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/9 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16037/ : 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/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 19:41:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21345 to look at the new patch set (#7). Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. IMPALA-12997: Use graceful shutdown for query log tests Uses graceful shutdown for all tests that might insert into 'sys.impala_query_log' to avoid leaving the table locked in HMS by a SIGTERM. That's primarily any test that sets 'query_log_write_interval_s' or 'query_log_max_queued'. Skips logging DFATAL if the local backend executor has not been added to an executor group, as happens on dedicated coordinators. Avoids Tried to remove non-existing backend from per-host list: 127.0.0.1:... and a crash with minidump when doing graceful shutdown on dedicated coordinators. Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc --- M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/executor-group.cc M be/src/scheduling/executor-group.h M tests/common/custom_cluster_test_suite.py M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py 6 files changed, 36 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/7 -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 19:09:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/21302/15/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21302/15/tests/custom_cluster/test_query_live.py@154 PS15, Line 154: # Drop table at the end, it's only recreated on impalad startup. > nit: I wonder what will happen in multi-coordinator cluster if this table i I haven't seen this be an issue. Same thing that would happen with any other table that one coordinator drops and another tries to access. -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 19:08:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/21302/15/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21302/15/tests/custom_cluster/test_query_live.py@154 PS15, Line 154: # Drop table at the end, it's only recreated on impalad startup. nit: I wonder what will happen in multi-coordinator cluster if this table is dropped from one coordinator and accessed from the other. If that is something that we need to address in the future (ie., prevent it from being dropped), please leave TODO comment. Otherwise, LGTM. -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 19:05:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 6: > Patch Set 6: > > Most of the tests modified here ran into > > Tried to remove non-existing backend from per-host list > > and crashed during graceful shutdown. I don't see any existing bugs filed for > this, so digging into it. All the failures come from dedicated coordinators going through graceful shutdown. Seems like a latent bug we haven't tested. -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 18:42:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16036/ : 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/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 18:36:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21345 ) Change subject: IMPALA-12997: Use graceful shutdown for query log tests .. Patch Set 6: Most of the tests modified here ran into Tried to remove non-existing backend from per-host list and crashed during graceful shutdown. I don't see any existing bugs filed for this, so digging into it. -- To view, visit http://gerrit.cloudera.org:8080/21345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc Gerrit-Change-Number: 21345 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 18:34:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13039: AES Encryption/ Decryption Support in Impala
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20447 ) Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/exprs/string-functions-ir.cc File be/src/exprs/string-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/exprs/string-functions-ir.cc@1901 PS9, Line 1901: delete t; Isn't this a memory leak if the code returns at line 1894? -- To view, visit http://gerrit.cloudera.org:8080/20447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 Gerrit-Change-Number: 20447 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 26 Apr 2024 18:21:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/21302/14/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21302/14/tests/custom_cluster/test_query_live.py@137 PS14, Line 137: insert_result = self.execute_query_expect_failure(self.client, : 'insert into sys.impala_query_live select * from sys.impala_query_live limit 1') > Because it's in HMS, drop table actually succeeds. It gets recreated the ne Done -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 18:13:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Hello Andrew Sherman, Quanlong Huang, Riza Suminto, Jason Fehr, Wenzhe Zhou, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21302 to look at the new patch set (#15). Change subject: IMPALA-13005: Create Query Live table in HMS .. IMPALA-13005: Create Query Live table in HMS Creates the 'sys.impala_query_live' table in HMS using a similar 'CREATE TABLE' command to 'sys.impala_query_log'. Updates frontend to identify a System Table based on the '__IMPALA_SYSTEM_TABLE' property. Tables improperly marked with '__IMPALA_SYSTEM_TABLE' will error when attempting to scan them because no relevant scanner will be available. Creating the table in HMS simplifies supporting 'SHOW CREATE TABLE' and 'DESCRIBE EXTENDED', so allows them for parity with Query Log. Explicitly disables 'COMPUTE STATS' on system tables as it doesn't work correctly. Makes System Tables work with local catalog mode, fixing LocalCatalogException: Unknown table type for table sys.impala_query_live Updates workload management implementation to rely more on SystemTables.thrift definition, and adds DCHECKs to verify completeness and ordering. Testing: - adds additional test cases for changes to introspection commands - passes existing test_query_live and test_query_log suites Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 --- M be/generated-sources/gen-cpp/CMakeLists.txt M be/src/exec/system-table-scanner.cc M be/src/service/workload-management-fields.cc M be/src/service/workload-management.cc M be/src/service/workload-management.h M common/thrift/CatalogObjects.thrift M common/thrift/SystemTables.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java A fe/src/main/java/org/apache/impala/analysis/SystemTableRef.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java A fe/src/main/java/org/apache/impala/catalog/FeSystemTable.java M fe/src/main/java/org/apache/impala/catalog/SystemTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java A fe/src/main/java/org/apache/impala/catalog/local/LocalSystemTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/catalog/SystemTableTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java D testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test M tests/custom_cluster/test_query_live.py 26 files changed, 435 insertions(+), 294 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/21302/15 -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-13012: Lower default query log max queued
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21351 ) Change subject: IMPALA-13012: Lower default query_log_max_queued .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10585/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21351 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6535675307d88cb65ba7d908f3c692e0cf3259d7 Gerrit-Change-Number: 21351 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 26 Apr 2024 18:15:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10584/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 18:14:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/21302/14/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/21302/14/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@a1560 PS14, Line 1560: This set of tests no longer worked because they require catalogd have enable_workload_mgmt=true. Previously SystemTables were handled entirely local and didn't interact with HMS/catalogd. -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 17:28:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13005: Create Query Live table in HMS
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21302 ) Change subject: IMPALA-13005: Create Query Live table in HMS .. Patch Set 14: (2 comments) http://gerrit.cloudera.org:8080/#/c/21302/14/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21302/14/tests/custom_cluster/test_query_live.py@132 PS14, Line 132: insert_result > nit: create_result? Ack http://gerrit.cloudera.org:8080/#/c/21302/14/tests/custom_cluster/test_query_live.py@137 PS14, Line 137: insert_result = self.execute_query_expect_failure(self.client, : 'insert into sys.impala_query_live select * from sys.impala_query_live limit 1') > nit: missing "drop table" test? Because it's in HMS, drop table actually succeeds. It gets recreated the next time Impala starts up. I could put one at the end of this case demonstrating that. -- To view, visit http://gerrit.cloudera.org:8080/21302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf302ee54a819fdee2db0ae582a5eeddffe4a5b4 Gerrit-Change-Number: 21302 Gerrit-PatchSet: 14 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Fri, 26 Apr 2024 17:18:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16035/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 26 Apr 2024 17:13:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters The error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte was matching with one of the two bytes that represented the '\u00FF' literal. A set containing all the special characters has been included and, '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. Testing: Some basic tests for both parquet and iceberg are provided in unicode-column-name.test. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 2 files changed, 79 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/9 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[native-toolchain-CR] IMPALA-12886: Bump GoogleTest version to 1.14.0
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21133 ) Change subject: IMPALA-12886: Bump GoogleTest version to 1.14.0 .. Patch Set 3: Code-Review+2 This looks good to me -- To view, visit http://gerrit.cloudera.org:8080/21133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa52b4809a7c969e863518b182d2df63256f4c43 Gerrit-Change-Number: 21133 Gerrit-PatchSet: 3 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:40:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13039: AES Encryption/ Decryption Support in Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20447 ) Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala .. Patch Set 9: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/16034/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/20447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 Gerrit-Change-Number: 20447 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Fri, 26 Apr 2024 16:42:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16033/ : 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/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:26:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13039: AES Encryption/ Decryption Support in Impala
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/20447 )
Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala
..
Patch Set 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util-test.cc
File be/src/util/openssl-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util-test.cc@68
PS9, Line 68: AES_CIPHER_MODE modes[] =
{impala::AES_CIPHER_MODE::AES_256_GCM, impala::AES_CIPHER_MODE::AES_256_CTR,
impala::AES_CIPHER_MODE::AES_256_CFB};
line too long (145 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util-test.cc@98
PS9, Line 98: AES_CIPHER_MODE modes[] =
{impala::AES_CIPHER_MODE::AES_256_GCM, impala::AES_CIPHER_MODE::AES_256_CTR,
impala::AES_CIPHER_MODE::AES_256_CFB};
line too long (143 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.h@154
PS9, Line 154: /// calling this function. If 'in' == 'out', the operation is
performed in-place; otherwise,
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.h@157
PS9, Line 157: Status Encrypt(const uint8_t* data, int64_t len, uint8_t* out,
int64_t* data_read = NULL) WARN_UNUSED_RESULT;
line too long (111 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.h@164
PS9, Line 164: Status Decrypt(const uint8_t* data, int64_t len, uint8_t* out,
int64_t* data_read = NULL) WARN_UNUSED_RESULT;
line too long (111 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.cc@201
PS9, Line 201: Status EncryptionKey::Encrypt(const uint8_t* data, int64_t len,
uint8_t* out, int64_t* data_read) {
line too long (99 > 90)
http://gerrit.cloudera.org:8080/#/c/20447/9/be/src/util/openssl-util.cc@205
PS9, Line 205: Status EncryptionKey::Decrypt(const uint8_t* data, int64_t len,
uint8_t* out, int64_t* data_read) {
line too long (99 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/20447
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9
Gerrit-Change-Number: 20447
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Fri, 26 Apr 2024 16:16:55 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13039: AES Encryption/ Decryption Support in Impala
pranav.lo...@cloudera.com has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/20447 ) Change subject: IMPALA-13039: AES Encryption/ Decryption Support in Impala .. IMPALA-13039: AES Encryption/ Decryption Support in Impala AES (Advanced Encryption Standard) crypto functions are widely recognized and respected encryption algorithm used to protect sensitive data which operate by transforming plaintext data into ciphertext using a symmetric key, ensuring confidentiality and integrity. This standard specifies the Rijndael algorithm, a symmetric block cipher that can process data blocks of 128 bits, using cipher keys with lengths of 128 and 256 bits. The patch makes use of the EVP_*() algorithms from the OpenSSL library. The patch includes: 1. AES-GCM, AES-ECB, AES-CTR, and AES-CFB encryption and decryption functionalities. 2. Support for both 128-bit and 256-bit key sizes for GCM and ECB modes. 3. Enhancements to EncryptionKey class to accommodate various AES modes. The aes_encrypt() and aes_decrypt() functions serve as entry points for encryption and decryption operations, handling encryption and decryption based on user-provided keys, AES modes, and initialization vectors (IVs). The implementation includes key length validation and IV vector size checks to ensure data integrity and confidentiality. Future Steps: 1. Design and implement support for gcm_tag in AES-GCM. 2. Add support for AAD (Additional Authenticated Data) in AES-GCM. 3. Implement function overloading for optional parameters in aes_encrypt and aes_decrypt functions. 4. Addition of 192 bit key length for various modes. Testing: The patch is thouroughly tested and the tests are included in exprs.test. Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 --- M be/src/exprs/string-functions-ir.cc M be/src/exprs/string-functions.h M be/src/util/openssl-util-test.cc M be/src/util/openssl-util.cc M be/src/util/openssl-util.h M common/function-registry/impala_functions.py M testdata/workloads/functional-query/queries/QueryTest/exprs.test 7 files changed, 528 insertions(+), 134 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/20447/9 -- To view, visit http://gerrit.cloudera.org:8080/20447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3902f2b1d95da4d06995cbd687e79c48e16190c9 Gerrit-Change-Number: 20447 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 8: Code-Review+2 Carry +2. -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:12:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10583/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:12:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:12:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 8: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/16032/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 8 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 26 Apr 2024 16:10:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/21340/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21340/7//COMMIT_MSG@13 PS7, Line 13: only)"). This patch adds check to recognize coordinator-only query in > nits: Done http://gerrit.cloudera.org:8080/#/c/21340/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/21340/7/be/src/scheduling/admission-controller.cc@998 PS7, Line 998: // Can't admit if: : // (a) There are already queued requests (and this is not admitting from the queue). : // (b) The resource pool is already at the maximum number of requests. : // (c) One of the executors or coordinator in 'schedule' is already at its maximum : // admission slots (when not using the default executor group). : // (d) There are not enough memory resources available for the query. > The comment could probably also be updated: Done -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 16:04:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Hello Abhishek Rawat, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21340
to look at the new patch set (#8).
Change subject: IMPALA-13024: Ignore slots if using default pool and empty group
..
IMPALA-13024: Ignore slots if using default pool and empty group
Slot based admission should not be enabled when using default pool.
There is a bug where coordinator-only query still does slot based
admission because executor group name set to
ClusterMembershipMgr::EMPTY_GROUP_NAME ("empty group (using coordinator
only)"). This patch adds check to recognize coordinator-only query in
default pool and skip slot based admission for it.
Testing:
- Add BE test AdmissionControllerTest.CanAdmitRequestSlotsDefault.
- In test_executor_groups.py, split test_coordinator_concurrency to
test_coordinator_concurrency_default and
test_coordinator_concurrency_two_exec_group_cluster to show the
behavior change.
- Pass core tests in ASAN build.
Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M tests/common/impala_connection.py
M tests/custom_cluster/test_executor_groups.py
9 files changed, 128 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/21340/8
--
To view, visit http://gerrit.cloudera.org:8080/21340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0
Gerrit-Change-Number: 21340
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13024: Ignore slots if using default pool and empty group
Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/21340 ) Change subject: IMPALA-13024: Ignore slots if using default pool and empty group .. Patch Set 7: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/21340/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21340/7//COMMIT_MSG@13 PS7, Line 13: only)"). This patch add check to recognize coordinator-only query at nits: add -> adds at -> in it from slot checking -> slot based admission for it http://gerrit.cloudera.org:8080/#/c/21340/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/21340/7/be/src/scheduling/admission-controller.cc@998 PS7, Line 998: // Can't admit if: : // (a) There are already queued requests (and this is not admitting from the queue). : // (b) The resource pool is already at the maximum number of requests. : // (c) One of the executors in 'schedule' is already at its maximum number of requests : // (when not using the default executor group). : // (d) There are not enough memory resources available for the query. The comment could probably also be updated: (c) One of the executors or coordinator in 'schedule' is already at its maximum admission slots (when not using the default executor group). -- To view, visit http://gerrit.cloudera.org:8080/21340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b08dea7ba0c78ac6b98c7a0b148df8fb036c4d0 Gerrit-Change-Number: 21340 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 26 Apr 2024 15:54:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters The error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte was matching with one of the two bytes that represented the '\u00FF' literal. A set containing all the special characters has been included and, '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. Testing: Some basic tests for both parquet and iceberg are provided in unicode-column-name.test. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 2 files changed, 79 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/8 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 8 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 8:
(15 comments)
> Uploaded patch set 8: Commit message was updated.
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@7
PS6, Line 7: IMPALA-11499: Refactor UrlEncode function to handle special
characters
> We don't usually break the title into two lines, and it is not too long eit
Done
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@8
PS6, Line 8:
> nit: let's put the title in one line
Done
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@12
PS6, Line 12: bytes
> Nit: surround with quotes ('specialCharacterMap').
Ack
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@13
PS6, Line 13: 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90',
> It's unclear to me how the unicode characters are handled incorrectly. E.g.
Ack
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@18
PS6, Line 18: '\xFF', which is an invalid UTF-8 byte is replaced
> '*out' was assigned to also before this change, so its old contents were di
Ack
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@23
PS6, Line 23: #include
> I don't think we need iostream.
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@46
PS6, Line 46: ':', '=', '?',
> Could also be static. Alternatively, we could put all these static function
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@55
PS6, Line 55: / cha
> The problem in the old code was that "\u00FF" translates to two bytes: [194
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@66
PS6, Line 66: }
> No need to clear it if it is assigned to at the end of the function.
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@74
PS6, Line 74:
:
> Could be misleading: does "not" also apply to "one of the commonly..."? If
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@80
PS6, Line 80:
> This is easier to understand if converted to a form without parentheses:
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@80
PS6, Line 80:
> It's an existing issue but I think we should cast 'ch' to unsigned char as
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@85
PS6, Line 85:
> nit: I think we can ignore explicitly using "std::" since std::uppercase is
Done
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@85
PS6, Line 85:
> We could put std::uppercase and std::hex after L68, before the loop, as the
Done
http://gerrit.cloudera.org:8080/#/c/21131/3/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
File
testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test:
http://gerrit.cloudera.org:8080/#/c/21131/3/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@298
PS3, Line 298:
> This comment has not been addressed.
Done
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Fri, 26 Apr 2024 15:51:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12973,IMPALA-11491,IMPALA-12651: Support BINARY nested in complex types in select list
Daniel Becker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21269 ) Change subject: IMPALA-12973,IMPALA-11491,IMPALA-12651: Support BINARY nested in complex types in select list .. IMPALA-12973,IMPALA-11491,IMPALA-12651: Support BINARY nested in complex types in select list Binary fields in complex types are currently not supported at all for regular tables (an error is returned). For Iceberg metadata tables, IMPALA-12899 added a temporary workaround to allow queries that contain these fields to succeed by NULLing them out. This change adds support for displaying them with base64 encoding for both regular and Iceberg metadata tables. Complex types are displayed in JSON format, so simply inserting the bytes of the binary fields is not acceptable as it would produce invalid JSON. Base64 is a widely used encoding that allows representing arbitrary binary information using only a limited set of ASCII characters. This change also adds support for top level binary columns in Iceberg metadata tables. However, these are not base64 encoded but are returned in raw byte format - this is consistent with how top level binary columns from regular (non-metadata) tables are handled. Testing: - added test queries in iceberg-metadata-tables.test referencing both nested and top level binary fields; also updated existing queries - moved relevant tests (queries extracting binary fields from within complex types) from nested-types-scanner-basic.test to a new binary-in-complex-type.test file and also added a query that selects the containing complex types; this new test file is run from test_scanners.py::TestBinaryInComplexType::\ test_binary_in_complex_type - moved negative tests in AnalyzerTest.TestUnsupportedTypes() to AnalyzeStmtsTest.TestComplexTypesInSelectList() and converted them to positive tests (expecting success); a negative test already in AnalyzeStmtsTest.TestComplexTypesInSelectList() was also converted Change-Id: I7b1d7fa332a901f05a46e0199e13fb841d2687c2 Reviewed-on: http://gerrit.cloudera.org:8080/21269 Tested-by: Impala Public Jenkins Reviewed-by: Csaba Ringhofer --- M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc M be/src/exec/iceberg-metadata/iceberg-metadata-scanner.h M be/src/exec/iceberg-metadata/iceberg-row-reader.cc M be/src/exec/iceberg-metadata/iceberg-row-reader.h M be/src/rpc/jni-thrift-util.h M be/src/runtime/complex-value-writer.inline.h M be/src/util/jni-util.cc M be/src/util/jni-util.h M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M testdata/data/README A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/data/0-0-data-danielbecker_20240408174043_c3737eaf-db30-4b88-aafb-f23c0f3c1dd3-job_17125053806420_0002-1-1.parquet A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/metadata/64da0e56-efa3-4025-bef1-1047fdd9a2b0-m0.avro A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/metadata/snap-3079551887386250470-1-64da0e56-efa3-4025-bef1-1047fdd9a2b0.avro A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/metadata/v1.metadata.json A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/metadata/v2.metadata.json A testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_with_key_metadata/metadata/version-hint.txt M testdata/datasets/functional/functional_schema_template.sql M testdata/datasets/functional/schema_constraints.csv A testdata/workloads/functional-query/queries/QueryTest/binary-in-complex-type.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test M testdata/workloads/functional-query/queries/QueryTest/nested-types-scanner-basic.test M tests/query_test/test_scanners.py 26 files changed, 441 insertions(+), 157 deletions(-) Approvals: Impala Public Jenkins: Verified Csaba Ringhofer: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/21269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7b1d7fa332a901f05a46e0199e13fb841d2687c2 Gerrit-Change-Number: 21269 Gerrit-PatchSet: 12 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs
[Impala-ASF-CR] IMPALA-12973,IMPALA-11491,IMPALA-12651: Support BINARY nested in complex types in select list
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21269 ) Change subject: IMPALA-12973,IMPALA-11491,IMPALA-12651: Support BINARY nested in complex types in select list .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21269 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b1d7fa332a901f05a46e0199e13fb841d2687c2 Gerrit-Change-Number: 21269 Gerrit-PatchSet: 11 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Comment-Date: Fri, 26 Apr 2024 12:51:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 6:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@7
PS6, Line 7: IMPALA-11499: Refactor UrlEncode function to handle special
We don't usually break the title into two lines, and it is not too long either.
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@12
PS6, Line 12: specialCharacterMap
Nit: surround with quotes ('specialCharacterMap').
http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@18
PS6, Line 18: Additionally, the function clears the output string
'*out' was assigned to also before this change, so its old contents were
discarded. This sentence could be removed.
http://gerrit.cloudera.org:8080/#/c/21131/3/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/3/be/src/util/coding-util.cc@48
PS3, Line 48: {'#', "%23"},
> Should be const.
This comment has not been addressed.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@23
PS6, Line 23: #include
I don't think we need iostream.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@46
PS6, Line 46: std::unordered_map
Could also be static. Alternatively, we could put all these static functions
and variables into an anonymous namespace, which is a more modern way of
ensuring that these are not visible outside this translation unit.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@55
PS6, Line 55: '\xFF
The problem in the old code was that "\u00FF" translates to two bytes:
[194,191]. Note that the notation '\u' is Java-specific. The process is
broken because the code takes the input bytes one by one.
The hanzi character '?', in UTF-8, is [232, 191, 144]. Its second character,
191, matched with the second character of "\u00FF" and that caused the problem.
I don't know why '\xFF' was included in the first place. If we'd like to handle
UTF-8 only, then '\xFF' is not a valid byte. If we'd like to handle non-UTF-8
bytes, then we should handle all other bytes > 127.
After some digging in Hive, I found this commit that expanded the list of
escaped characters:
https://github.com/apache/hive/commit/32b046bf93e3d041b2bb07a1c9b4e1ef5d977ddf
The list before that commit is very similar to what we have here in the old
code, except they have '\u007F' instead of '\u00FF'. That makes more sense as
that is a control character for DELETE, see
https://www.compart.com/en/unicode/U+007F.
If we remove '\xFF', we can see that the escaped values in the map are the same
as how we HEX encode values on L85, so there is no need for these values to be
included in a map. Instead, we could have a set (std::unordered_set) with the
keys in the current map (without '\xFF' but including '\x7F'), and we would
encode input characters if they are in the set (taking into account hive_compat
of course). That would also simplify the code.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@66
PS6, Line 66: (*out).clear();
No need to clear it if it is assigned to at the end of the function.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@69
PS6, Line 69: for (int i = 0; i < in_len; ++i) {
Could use a range-based for-loop: for (char ch : in) {...}.
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@74
PS6, Line 74: is not
: // alphanumeric or one of the commonly excluded characters
Could be misleading: does "not" also apply to "one of the commonly..."? If not,
we could write for example "is not alphanumeric or is one of the commonly
excluded ...".
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@80
PS6, Line 80: !(std::isalnum(ch) || ShouldNotEscape(ch))
This is easier to understand if converted to a form without parentheses:
!std::isalnum(ch) && !ShouldNotEscape(ch)
http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@85
PS6, Line 85: std::uppercase << std::hex
We could put std::uppercase and std::hex after L68, before the loop, as these
settings don't get reset automatically, so no need to set them every time. On
the other hand, I would include a comment stating that this only applies to
when integer values are inserted and not when char values, therefore it doesn't
cause a problem when not escaping characters (like on L88).
http://gerrit.cloudera.org:8080/#/c/21131/3/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
File
testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test:
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@8 PS6, Line 8: characters nit: let's put the title in one line http://gerrit.cloudera.org:8080/#/c/21131/6//COMMIT_MSG@13 PS6, Line 13: that accurately maps special characters to their URL-encoded forms. It's unclear to me how the unicode characters are handled incorrectly. E.g. the one mentioned in the JIRA description is "?" which is encoded into 3 bytes in UTF-8: 0xe8 0xbf 0x90. Could you mention in the commit message how this example fails before and works now? FWIW, there are online tools to convert Unicode to UTF-8, e.g. https://onlinetools.com/unicode/convert-unicode-to-utf8 http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@80 PS6, Line 80: std::isalnum(ch) It's an existing issue but I think we should cast 'ch' to unsigned char as memtioned in https://en.cppreference.com/w/cpp/string/byte/isalnum > the behavior of std::isalnum is undefined if the argument's value is neither > representable as unsigned char nor equal to EOF. To use these functions > safely with plain chars (or signed chars), the argument should first be > converted to unsigned char: > std::isalnum(static_cast(ch)); http://gerrit.cloudera.org:8080/#/c/21131/6/be/src/util/coding-util.cc@85 PS6, Line 85: std:: nit: I think we can ignore explicitly using "std::" since std::uppercase is introduced at L37. For std::hex, it's introduced in "common/names.h": https://github.com/apache/impala/blob/b39cd79ae84c415e0aebec2c2b4d7690d2a0cc7a/be/src/common/names.h#L82 Maybe the same for "std::isalnum". -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 6 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Fri, 26 Apr 2024 09:03:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]Hierarchical metastore event processing
cclive1...@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21031 ) Change subject: [WIP]Hierarchical metastore event processing .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/21031/12/fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java File fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java: http://gerrit.cloudera.org:8080/#/c/21031/12/fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java@213 PS12, Line 213: eventProcessor_.getStatus()); of we can just mark this log as debug is debug is enabled. -- To view, visit http://gerrit.cloudera.org:8080/21031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76d8a739f9db6d40f01028bfd786a85d83f9e5d6 Gerrit-Change-Number: 21031 Gerrit-PatchSet: 12 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 26 Apr 2024 07:50:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP]Hierarchical metastore event processing
cclive1...@gmail.com has posted comments on this change. (
http://gerrit.cloudera.org:8080/21031 )
Change subject: [WIP]Hierarchical metastore event processing
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21031/12/fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java
File fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java:
http://gerrit.cloudera.org:8080/#/c/21031/12/fe/src/main/java/org/apache/impala/catalog/events/TableEventExecutor.java@212
PS12, Line 212: LOG.info("[{}] Clear event processor since status is {}",
executorName_,
I think we can remove this log or using some rate log method to print log ,I
tested this code ,and I got many unused logs, which will use much storage (it
printed 5 log files (200M/per file) just in 5min), and most of them a useless
and invalid.
--
To view, visit http://gerrit.cloudera.org:8080/21031
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76d8a739f9db6d40f01028bfd786a85d83f9e5d6
Gerrit-Change-Number: 21031
Gerrit-PatchSet: 12
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 26 Apr 2024 07:46:03 +
Gerrit-HasComments: Yes
