[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10400/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 05:57:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 05:57:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 05:57:22 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 4: > Be sure to rebase this, as we have been batching up a few changes into > native-toolchain to go into the next toolchain build for upstream Impala. Sure. I'm verifying the latest patch set. -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 05:27:57 + Gerrit-HasComments: No
[native-toolchain-CR] PROTOTYPE: Build shared libs for googletest
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21177 Change subject: PROTOTYPE: Build shared libs for googletest .. PROTOTYPE: Build shared libs for googletest This also bumps googletest to 1.14.0 Change-Id: I817a7721f12053540e06669402d97a69588ac0e9 --- M buildall.sh M source/googletest/build.sh M source/orc/build.sh 3 files changed, 17 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/77/21177/1 -- To view, visit http://gerrit.cloudera.org:8080/21177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I817a7721f12053540e06669402d97a69588ac0e9 Gerrit-Change-Number: 21177 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 3: Be sure to rebase this, as we have been batching up a few changes into native-toolchain to go into the next toolchain build for upstream Impala. -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 04:58:51 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21161/2/source/gtest/build.sh File source/gtest/build.sh: http://gerrit.cloudera.org:8080/#/c/21161/2/source/gtest/build.sh@49 PS2, Line 49: in.so build_static/libgtest.a > Nit: There is also a libgtest_main.so produced by the shared build. Should OK, added this file. -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 04:52:50 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 3: Code-Review+2 This looks good to me -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 04:51:52 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 2: Code-Review+1 > (1 comment) > > > We've got a couple things in flight. > > > > This change to build shared libraries for gtest makes sense. > > > > Laszlo has a change to bump the googletest version here: > https://gerrit.cloudera.org/#/c/21133/ (with a corresponding Impala > change to switch to the new version). We should do an equivalent > thing for source/googletest/build.sh, but that might fit better in > Laszlo's change. > > I think Laszlo's patch also wants to fix the same issue. But I'm > not sure whether googletest-1.14 has global variables. If there are > still global variables, the crash due to double-free memory issue > could still occur. I think this patch and > https://gerrit.cloudera.org/c/21163/ > fix the issue directly. This is better than what we have, so we can go ahead with this. -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 04:51:05 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Hello Laszlo Gaal, Riza Suminto, Zoltan Borok-Nagy, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21161 to look at the new patch set (#3). Change subject: IMPALA-12915: Build shared library of GTest .. IMPALA-12915: Build shared library of GTest When building Impala with shared libraries, we currently link against the static library of GTest in both libkudu_test_util.so and unifiedbetests. libkudu_test_util.so is also linked by unifiedbetests. When unifiedbetests exits, both binaries try to delete the global variables of GTest, which leads to a double-free memory issue. This patch builds both the shared and static library of GTest so Impala can pick the corresponding library correctly. Test: - Built GTest locally and verify the issue in Impala got resolved Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 --- M source/gtest/build.sh 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/61/21161/3 -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-12916: Fix test event processor error global invalidate test random failure
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21169 ) Change subject: IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure .. IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure Event processor goes to error state before it tries to global invalidate It remains in error state for a very short period of time. If wait_for_synced_event_id() obtains event processor status during this period, it can get status as error. This test was introduced with IMPALA-12832. Testing: - Tested manually. Added sleep in code for testing so that event processor remains in error state for little longer time. Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Reviewed-on: http://gerrit.cloudera.org:8080/21169 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_event_processing_error.py M tests/util/event_processor_utils.py 2 files changed, 12 insertions(+), 7 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Gerrit-Change-Number: 21169 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12916: Fix test event processor error global invalidate test random failure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21169 ) Change subject: IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Gerrit-Change-Number: 21169 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 04:50:52 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21161/2/source/gtest/build.sh File source/gtest/build.sh: http://gerrit.cloudera.org:8080/#/c/21161/2/source/gtest/build.sh@49 PS2, Line 49: build_static/libgtest_main.a\ Nit: There is also a libgtest_main.so produced by the shared build. Should we include that? The corresponding Impala change seems to be ok without it, but maybe for completeness? -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 04:32:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12881: Use getFkPkJoinCardinality to reduce scan cardinality
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21118 ) Change subject: IMPALA-12881: Use getFkPkJoinCardinality to reduce scan cardinality .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21118/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: http://gerrit.cloudera.org:8080/#/c/21118/5/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@833 PS5, Line 833: scanCardinality; nit: don't need to assign this initial value -- To view, visit http://gerrit.cloudera.org:8080/21118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6efafffc8f96247a860b88e85d9097b2b4327f32 Gerrit-Change-Number: 21118 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 21 Mar 2024 04:31:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21173 ) Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 21 Mar 2024 04:04:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21173 ) Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10399/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 21 Mar 2024 04:05:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21173 ) Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 21 Mar 2024 04:05:22 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-12915: Use libgtest.so when built with shared libs
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21163 ) Change subject: WIP IMPALA-12915: Use libgtest.so when built with shared libs .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21163/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21163/2//COMMIT_MSG@7 PS2, Line 7: WIP nit: drop WIP? -- To view, visit http://gerrit.cloudera.org:8080/21163 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2 Gerrit-Change-Number: 21163 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 21 Mar 2024 04:03:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12786: Optimize count(*) for JSON scans
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21039 )
Change subject: IMPALA-12786: Optimize count(*) for JSON scans
..
Patch Set 4:
(5 comments)
Thank you for filing this patch. I have some questions and comments.
http://gerrit.cloudera.org:8080/#/c/21039/4/be/src/exec/json/json-parser.cc
File be/src/exec/json/json-parser.cc:
http://gerrit.cloudera.org:8080/#/c/21039/4/be/src/exec/json/json-parser.cc@37
PS4, Line 37: This class is
: /// essentially a simplified version of a rapidjson parser,
removing specific data parsing
: /// and copying operations, allowing for faster parsing of the
number of JSON objects.
Can you put reference link to rapidjson code where this class is based from?
http://gerrit.cloudera.org:8080/#/c/21039/4/be/src/exec/json/json-parser.cc@45
PS4, Line 45: class JsonSkipper {
Can you add more tests for this class?
Please test for all possible positive cases where skipping successful and
negative cases where skipping fail and return error.
http://gerrit.cloudera.org:8080/#/c/21039/4/be/src/exec/json/json-parser.cc@126
PS4, Line 126: bool SkipNumber() {
: Consume('-');
Can you add comment about valid number literal? What if it begin with '.' or
'+' ?
http://gerrit.cloudera.org:8080/#/c/21039/4/be/src/exec/json/json-parser.cc@182
PS4, Line 182: case 'n': RETURN_IF_FALSE(SkipNull()); break;
: case 't': RETURN_IF_FALSE(SkipTrue()); break;
: case 'f': RETURN_IF_FALSE(SkipFalse()); break;
Is s_ always lower case?
http://gerrit.cloudera.org:8080/#/c/21039/4/testdata/workloads/functional-query/queries/QueryTest/malformed_json.test
File testdata/workloads/functional-query/queries/QueryTest/malformed_json.test:
http://gerrit.cloudera.org:8080/#/c/21039/4/testdata/workloads/functional-query/queries/QueryTest/malformed_json.test@27
PS4, Line 27: select count(*) from malformed_json
: TYPES
: bigint
: RESULTS
: 13
Is the same query yield the same result before this patch?
--
To view, visit http://gerrit.cloudera.org:8080/21039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97ff097661c3c577aeafeeb1518408ce7a8a255e
Gerrit-Change-Number: 21039
Gerrit-PatchSet: 4
Gerrit-Owner: Zihao Ye
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Thu, 21 Mar 2024 03:58:30 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 2: (1 comment) > We've got a couple things in flight. > > This change to build shared libraries for gtest makes sense. > > Laszlo has a change to bump the googletest version here: > https://gerrit.cloudera.org/#/c/21133/ (with a corresponding Impala change to > switch to the new version). We should do an equivalent thing for > source/googletest/build.sh, but that might fit better in Laszlo's change. I think Laszlo's patch also wants to fix the same issue. But I'm not sure whether googletest-1.14 has global variables. If there are still global variables, the crash due to double-free memory issue could still occur. I think this patch and https://gerrit.cloudera.org/c/21163/ fix the issue directly. http://gerrit.cloudera.org:8080/#/c/21161/1/source/gtest/build.sh File source/gtest/build.sh: http://gerrit.cloudera.org:8080/#/c/21161/1/source/gtest/build.sh@42 PS1, Line 42: .. > Nit: BUILD_STATIC_LIBS doesn't do anything. The default is to build static Removed this. -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 03:45:09 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Hello Laszlo Gaal, Riza Suminto, Zoltan Borok-Nagy, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21161 to look at the new patch set (#2). Change subject: IMPALA-12915: Build shared library of GTest .. IMPALA-12915: Build shared library of GTest When building Impala with shared libraries, we currently link against the static library of GTest in both libkudu_test_util.so and unifiedbetests. libkudu_test_util.so is also linked by unifiedbetests. When unifiedbetests exits, both binaries try to delete the global variables of GTest, which leads to a double-free memory issue. This patch builds both the shared and static library of GTest so Impala can pick the corresponding library correctly. Test: - Built GTest locally and verify the issue in Impala got resolved Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 --- M source/gtest/build.sh 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/61/21161/2 -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21174 ) Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15600/ : 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/21174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 Gerrit-Change-Number: 21174 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Thu, 21 Mar 2024 02:17:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21174 )
Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py@54
PS1, Line 54: Core/pairwise only deals with precision
> Not familiar with the "pairwise" mode, is it the same as the core now?
Yes, core is permuted the same as pairwise now.
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py@109
PS1, Line 109: precision = vector.get_value('precision')
> This line can be removed?
Done
--
To view, visit http://gerrit.cloudera.org:8080/21174
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2
Gerrit-Change-Number: 21174
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Thu, 21 Mar 2024 01:55:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Hello Yida Wu, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21174 to look at the new patch set (#2). Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py .. IMPALA-4545: Simplify test dimension in test_decimal_casting.py This patch split precision and scale as their own dimension and then constraint them to yield valid decimal type. Also done minor refactoring to reduce test skipping and pass flake8. After this patch, core exploration has 214 test items and exhaustive exploration has 12312 test items. Before, they were 408 and 12464 respectively. Testing: - Pass test_decimal_casting.py in core and exhaustive exploration. Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 --- M tests/query_test/test_decimal_casting.py 1 file changed, 48 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/21174/2 -- To view, visit http://gerrit.cloudera.org:8080/21174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 Gerrit-Change-Number: 21174 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Yida Wu
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Query Live Table .. Patch Set 43: (2 comments) http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift File common/thrift/SystemTables.thrift: http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift@21 PS41, Line 21: # Must be kept in-s > SystemTable is a generic concept that may be used for other things (like a TQueryTableColumn sounds good. http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java File fe/src/main/java/org/apache/impala/catalog/SystemTable.java: http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@52 PS41, Line 52: private static final Map SYSTEM_TABLE_NAME_MAP = : ImmutableMap.of(QUERY_LIVE, TSystemTableName.QUERY_LIVE); > This is an ImmutableMap. I'm confused by your comment Riza. Ah, nevermind. I misread it the first time. The FE test ask is still relevant though. -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 43 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: Thu, 21 Mar 2024 01:49:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Yida Wu has posted comments on this change. (
http://gerrit.cloudera.org:8080/21174 )
Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py
..
Patch Set 1: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py@54
PS1, Line 54: Core only deals with precision 6, 16, 26
Not familiar with the "pairwise" mode, is it the same as the core now?
http://gerrit.cloudera.org:8080/#/c/21174/1/tests/query_test/test_decimal_casting.py@109
PS1, Line 109: # precision, scale = vector.get_value('decimal_type')
This line can be removed?
--
To view, visit http://gerrit.cloudera.org:8080/21174
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2
Gerrit-Change-Number: 21174
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Yida Wu
Gerrit-Comment-Date: Thu, 21 Mar 2024 01:47:50 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 1: (1 comment) We've got a couple things in flight. This change to build shared libraries for gtest makes sense. Laszlo has a change to bump the googletest version here: https://gerrit.cloudera.org/#/c/21133/ (with a corresponding Impala change to switch to the new version). We should do an equivalent thing for source/googletest/build.sh, but that might fit better in Laszlo's change. http://gerrit.cloudera.org:8080/#/c/21161/1/source/gtest/build.sh File source/gtest/build.sh: http://gerrit.cloudera.org:8080/#/c/21161/1/source/gtest/build.sh@42 PS1, Line 42: -DBUILD_STATIC_LIBS=ON Nit: BUILD_STATIC_LIBS doesn't do anything. The default is to build static libraries and BUILD_STATIC_LIBS is ignored as a unknown variable. (It doesn't cause any harm though.) -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Mar 2024 00:55:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12829: Skip processing transaction events if the table is blacklisted
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21175 ) Change subject: IMPALA-12829: Skip processing transaction events if the table is blacklisted .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15599/ : 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/21175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019 Gerrit-Change-Number: 21175 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 21 Mar 2024 00:53:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12829: Skip processing transaction events if the table is blacklisted
Sai Hemanth Gantasala has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21175 Change subject: IMPALA-12829: Skip processing transaction events if the table is blacklisted .. IMPALA-12829: Skip processing transaction events if the table is blacklisted For transactional tables, the event processor is not skipping abort_txn and alloc_write_id_event if the database/table is blacklisted. This processing is unnecessary and helps to improve event processor lag by skipping abort_txn, commit_txn and alloc_write_id_event events if the corresponding transactional tables are blacklisted. Testing: - Added end-to-end tests to verify transaction events are skipped. Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M tests/custom_cluster/test_events_custom_configs.py 2 files changed, 77 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/21175/1 -- To view, visit http://gerrit.cloudera.org:8080/21175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019 Gerrit-Change-Number: 21175 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/21109/5/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/21109/5/fe/pom.xml@616 PS5, Line 616: : org.apache.calcite : calcite-core : 1.36.0 : : : org.apache.calcite.avatica : avatica-core : 1.23.0 : > Yeah. This was an interesting dilemma for me Ok, I think I made the changes that make this better. The dependencies are now in the java/calcite-planner/target directory and the path is set up via set-classpath through an environment variable. The CLASSPATH is set up similar to how fe sets up the CLASSPATH. The custom cluster unit test is now changed too. -- To view, visit http://gerrit.cloudera.org:8080/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Thu, 21 Mar 2024 00:31:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 ) Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15598/ : 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/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 10 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 00:34:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Query Live Table .. Patch Set 43: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15597/ : 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/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 43 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: Thu, 21 Mar 2024 00:34:22 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-12832: Test jenkins tests
Sai Hemanth Gantasala has abandoned this change. ( http://gerrit.cloudera.org:8080/21069 ) Change subject: [WIP] IMPALA-12832: Test jenkins tests .. Abandoned Patch is already merged via another gerrit -- To view, visit http://gerrit.cloudera.org:8080/21069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I1d7ab7c46e039b0a23d9b998eff7940bce763b1d Gerrit-Change-Number: 21069 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Query Live Table .. Patch Set 42: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15596/ : 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/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 42 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: Thu, 21 Mar 2024 00:23:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12264: Add limit on number of HS2 sessions per user.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21128 ) Change subject: IMPALA-12264: Add limit on number of HS2 sessions per user. .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15595/ : 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/21128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd28edc352102d89774f6ece5376e7c79ae41aa8 Gerrit-Change-Number: 21128 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Comment-Date: Thu, 21 Mar 2024 00:22:27 + Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-12905: Disk-based tuple caching
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21171 ) Change subject: PROTOTYPE: IMPALA-12905: Disk-based tuple caching .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf Gerrit-Change-Number: 21171 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Thu, 21 Mar 2024 00:22:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Hello Quanlong Huang, k.venureddy2...@gmail.com, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21019 to look at the new patch set (#10). Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor IMPALA-11534 skips reloading file metadata for some trivial ALTER_TABLE events. However, ALTER_TABLE events that have trivial changes in StorageDescriptor are not handled in IMPALA-11534. The only changes that require file metadata reload are: location, rowformat, fileformat, serde, and storedAsSubDirectories. The file metadata reload can be skipped for all other changes in SD. Testing: 1) Manual testing by changing SD parameters in local environment. 2) Added unit tests for the same in MetastoreEventsProcessorTest class. Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac --- M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 5 files changed, 173 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/21019/10 -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 10 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Sai Hemanth Gantasala has posted comments on this change. (
http://gerrit.cloudera.org:8080/21019 )
Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE
events with trivial changes in StorageDescriptor
..
Patch Set 9:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1899
PS8, Line 1899: (isTrivialSdPropsChang
> Can't this return true when the SD didn't change at all but there are other
Good point! This is what I'm missing. We should check for SD changes only if we
notice any changes in SD.
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1963
PS8, Line 1963:
> Is it possible that on is null while the other is not? Shouldn't the functi
I have addressed this at L#1899
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1965
PS8, Line 1965: // Check if the trivial SD properties are changed during
the alter statement
> nit: this looks a bit unusual in Impala, can you split it to to two lines?
Ack
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1967
PS8, Line 1967: StorageDescriptor afterSd) {
> non-trivial could be added to the log message
Done
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1977
PS8, Line 1977:
> I think that it would make the intention of the function clearer to just st
Ack
http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1987
PS8, Line 1987: sd.unsetCols();
> Can you add a comment for this? The previous lines just unset the propertie
Done
--
To view, visit http://gerrit.cloudera.org:8080/21019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac
Gerrit-Change-Number: 21019
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sai Hemanth Gantasala
Gerrit-Comment-Date: Thu, 21 Mar 2024 00:12:29 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12856: Event processor should ignore processing partition with empty partition values
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21143 ) Change subject: IMPALA-12856: Event processor should ignore processing partition with empty partition values .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21143/3//COMMIT_MSG@7 PS3, Line 7: IMPALA-12856: Event processor should ignore processing partition > Have checked org.apache.hadoop.hive.metastore.MetaStoreDirectSql#getPartiti IMO, we can use this patch to address the empty partition values from HMS as we have seen this in production env. Other discovered cases should be handled in metastore as handling them in the catalog's cache becomes complex. -- To view, visit http://gerrit.cloudera.org:8080/21143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2469930ccd74948325f1723bd8b2bd6aad02d09 Gerrit-Change-Number: 21143 Gerrit-PatchSet: 3 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 00:11:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15593/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 10 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Thu, 21 Mar 2024 00:13:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 ) Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15594/ : 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/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 9 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Thu, 21 Mar 2024 00:12:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Hello Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20762 to look at the new patch set (#43). Change subject: IMPALA-12540: Query Live Table .. IMPALA-12540: Query Live Table Defines SystemTable which are in-memory tables that can provide access to Impala state. Adds the 'impala_query_live' to the database 'sys', which already exists for 'sys.impala_query_log'. Implements the 'impala_query_live' table to view active queries across all coordinators sharing the same statestore. SystemTables create new SystemTableScanNodes for their scan node implementation. When computing scan range locations, SystemTableScanNodes creates a scan range for each in the cluster (identified via ClusterMembershipMgr). This produces a plan that looks like: Query: explain select * from sys.impala_query_live ++ | Explain String | ++ | Max Per-Host Resource Reservation: Memory=4.00MB Threads=2 | | Per-Host Resource Estimates: Memory=11MB | | WARNING: The following tables are missing relevant table | | and/or column statistics. | | sys.impala_query_live | || | PLAN-ROOT SINK | | | | | 01:EXCHANGE [UNPARTITIONED]| | | | | 00:SCAN SYSTEM_TABLE [sys.impala_query_live] | |row-size=72B cardinality=20 | ++ Execution will pull data from ImpalaServer on the backend via a SystemTableScanner implementation based on table name. In the query profile, SYSTEM_TABLE_SCAN_NODE includes ActiveQueryCollectionTime and PendingQueryCollectionTime to track time spent collecting QueryState from ImpalaServer. Grants QueryScanner private access to ImpalaServer, identical to how ImpalaHttpHandler access internal server state. Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 --- M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc M be/src/exec/scan-node.cc A be/src/exec/system-table-scan-node.cc A be/src/exec/system-table-scan-node.h A be/src/exec/system-table-scanner.cc A be/src/exec/system-table-scanner.h M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/scheduling/cluster-membership-mgr-test.cc M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/cluster-membership-mgr.h M be/src/scheduling/scheduler.cc M be/src/service/fe-support.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record.h M be/src/service/workload-management-fields.cc M be/src/service/workload-management.cc M be/src/util/sharded-query-map-util.h M common/thrift/CMakeLists.txt M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M common/thrift/PlanNodes.thrift A common/thrift/SystemTables.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.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/SystemTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java A fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java A testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test A tests/custom_cluster/test_query_live.py M tests/util/workload_management.py 45 files changed, 1,377 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/20762/43 -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Hello Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20762 to look at the new patch set (#42). Change subject: IMPALA-12540: Query Live Table .. IMPALA-12540: Query Live Table Defines SystemTable which are in-memory tables that can provide access to Impala state. Adds the 'impala_query_live' to the database 'sys', which already exists for 'sys.impala_query_log'. Implements the 'impala_query_live' table to view active queries across all coordinators sharing the same statestore. SystemTables create new SystemTableScanNodes for their scan node implementation. When computing scan range locations, SystemTableScanNodes creates a scan range for each in the cluster (identified via ClusterMembershipMgr). This produces a plan that looks like: Query: explain select * from sys.impala_query_live ++ | Explain String | ++ | Max Per-Host Resource Reservation: Memory=4.00MB Threads=2 | | Per-Host Resource Estimates: Memory=11MB | | WARNING: The following tables are missing relevant table | | and/or column statistics. | | sys.impala_query_live | || | PLAN-ROOT SINK | | | | | 01:EXCHANGE [UNPARTITIONED]| | | | | 00:SCAN SYSTEM_TABLE [sys.impala_query_live] | |row-size=72B cardinality=20 | ++ Execution will pull data from ImpalaServer on the backend via a SystemTableScanner implementation based on table name. In the query profile, SYSTEM_TABLE_SCAN_NODE includes ActiveQueryCollectionTime and PendingQueryCollectionTime to track time spent collecting QueryState from ImpalaServer. Grants QueryScanner private access to ImpalaServer, identical to how ImpalaHttpHandler access internal server state. Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 --- M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc M be/src/exec/scan-node.cc A be/src/exec/system-table-scan-node.cc A be/src/exec/system-table-scan-node.h A be/src/exec/system-table-scanner.cc A be/src/exec/system-table-scanner.h M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/scheduling/cluster-membership-mgr-test.cc M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/cluster-membership-mgr.h M be/src/scheduling/scheduler.cc M be/src/service/fe-support.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record.h M be/src/service/workload-management-fields.cc M be/src/service/workload-management.cc M be/src/util/sharded-query-map-util.h M common/thrift/CMakeLists.txt M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M common/thrift/PlanNodes.thrift A common/thrift/SystemTables.thrift M common/thrift/Types.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.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/SystemTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java A fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java A testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test A tests/custom_cluster/test_query_live.py M tests/util/workload_management.py 45 files changed, 1,377 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/20762/42 -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Query Live Table .. Patch Set 42: (1 comment) http://gerrit.cloudera.org:8080/#/c/20762/42/be/src/scheduling/cluster-membership-mgr-test.cc File be/src/scheduling/cluster-membership-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/20762/42/be/src/scheduling/cluster-membership-mgr-test.cc@817 PS42, Line 817: vector orig_coordinators = line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 42 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: Thu, 21 Mar 2024 00:03:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12264: Add limit on number of HS2 sessions per user.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/21128 ) Change subject: IMPALA-12264: Add limit on number of HS2 sessions per user. .. Patch Set 3: (12 comments) Thanks for the thoughtful review http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@376 PS2, Line 376: key = > nit: remove this since already declared at L359 Ignoring as I moved the earlier declaration. http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@377 PS2, Line 377: } > Should we decrement the count if we return here? Good question thanks. I have moved where I do the check. I think the undocumented structure of the method is that various checks are performed, and then later information about the session is persisted. I moved my new check of session counts to be at the end of the section where checks are done. http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@388 PS2, Line 388: > nit: remove this since already declared at L359 Ignoring as I moved the earlier declaration. http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@425 PS2, Line 425: > Same question - should we decrease the count if we return here? Or maybe we Good question again. See earlier for how I am dealing with this http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@465 PS2, Line 465: DCHECK(false) << msg; > This seems a bug if the session is opened successfully. Could you add a DCH Done http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@475 PS2, Line 475: // Don't allow decrement below zero. > nit: also add a warning log for this? Done http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@481 PS2, Line 481: > nit: const string& Done http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-hs2-server.cc@488 PS2, Line 488: > nit: const string& Done http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-http-handler.cc File be/src/service/impala-http-handler.cc: http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-http-handler.cc@796 PS2, Line 796: sort(users.Begin(), users.End(), SessionCountComparer); > nit: don't need to hold the lock during sorting Yes, thanks http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-server.cc@265 PS2, Line 265: by any single connected user > nit: let's add "on a coordinator" to emphasize this is a local limit. Done http://gerrit.cloudera.org:8080/#/c/21128/2/be/src/service/impala-server.cc@2715 PS2, Line 2715: } > Should we move this into the above scope protected by session_state->lock ? Thanks, good catch. When I wrote this I meant to put my Decrement calls near where IMPALA_SERVER_NUM_OPEN_HS2_SESSIONS is decremented, not sure why I didn't do that but I am happier now. http://gerrit.cloudera.org:8080/#/c/21128/2/tests/custom_cluster/test_session_expiration.py File tests/custom_cluster/test_session_expiration.py: http://gerrit.cloudera.org:8080/#/c/21128/2/tests/custom_cluster/test_session_expiration.py@197 PS2, Line 197: assert res['users'][0]['session_count'] == 2 > Can we also verify the count finally come down to 0, e.g. by adding a sleep Thanks, done -- To view, visit http://gerrit.cloudera.org:8080/21128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd28edc352102d89774f6ece5376e7c79ae41aa8 Gerrit-Change-Number: 21128 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Comment-Date: Wed, 20 Mar 2024 23:58:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12264: Add limit on number of HS2 sessions per user.
Andrew Sherman has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21128 ) Change subject: IMPALA-12264: Add limit on number of HS2 sessions per user. .. IMPALA-12264: Add limit on number of HS2 sessions per user. Add a new flag --max_hs2_sessions_per_user which sets a limit on the number of Hiveserver2 sessions that can be concurrently opened by a user on a coordinator. By default this value is -1, which disables the new feature. An attempt to open more sessions than the new flag value results in an error. This is implemented by maintaining a map of users to a count of HS2 sessions. If the per-user HS2 session counts are being limited in this way, then the per-user counts are visible on the /sessions page of the webui. Add a new test case in test_session_expiration.py. Change-Id: Idd28edc352102d89774f6ece5376e7c79ae41aa8 --- M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M tests/custom_cluster/test_session_expiration.py M www/sessions.tmpl 7 files changed, 184 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/21128/3 -- To view, visit http://gerrit.cloudera.org:8080/21128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idd28edc352102d89774f6ece5376e7c79ae41aa8 Gerrit-Change-Number: 21128 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table
..
Patch Set 41:
(32 comments)
Upload incoming. I still need to add some of the requested tests.
http://gerrit.cloudera.org:8080/#/c/20762/41//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20762/41//COMMIT_MSG@19
PS41, Line 19:
: Query: explain select * from sys.impala_query_live
> Consider adding planner test for queries against sys.impala_query_live.
Done.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h
File be/src/exec/system-table-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h@40
PS41, Line 40: by fetching more data from metrics.
> Nit: this comment does not make sense in this context. Possibly a copy-and
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.h
File be/src/exec/system-table-scanner.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.h@42
PS41, Line 42: bool eos() const { return eos_; }
> nit: add noexcept?
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@26
PS41, Line 26: #include "runtime/decimal-value.h"
> unused
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@27
PS41, Line 27: #include "runtime/decimal-value.inline.h"
> unused
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@71
PS41, Line 71: MaterializeNextRow
> WriteStringSlot?
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/fe-support.cc@655
PS41, Line 655: for (const auto& it : membership_snapshot->current_backends) {
: if (it.second.has_is_coordinator() &&
it.second.is_coordinator()) {
: VLOG_QUERY << "Found coordinator "
: << it.second.address().hostname() << ":" <<
it.second.address().port();
:
coordinators.emplace_back(FromNetworkAddressPB(it.second.address()));
: }
: }
> What do you think about pushing this to ClusterMembershipMgr or ClusterMemb
Makes sense, I've moved it there.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@509
PS41, Line 509: size_t NumLiveQueries();
> Please mark this function const noexcept.
It's not though. It has to take a lock (which is not marked mutable). Locking
can throw an exception.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@508
PS41, Line 508: /// Return the number of live queries managed by this server.
: size_t NumLiveQueries();
> mention that this is implemented in workload-management.cc
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@1138
PS41, Line 1138: /// Returns a list of completed queries that have not yet
been written to storage.
> Please document that this function makes a copy of the completed queries qu
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@1139
PS41, Line 1139: std::vector>
GetCompletedQueries();
> Please mark this function const noexcept. The completed_queries_lock_ will
Do we really want to do that? lock() can throw an exception. We do occasionally
mark locks as mutable, but not consistently. What's the rationale for doing so
here?
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@1138
PS41, Line 1138: /// Returns a list of completed queries that have not yet been
written to storage.
: std::vector>
GetCompletedQueries();
> mention that this is implemented in workload-management.cc
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/util/sharded-query-map-util.h
File be/src/util/sharded-query-map-util.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/util/sharded-query-map-util.h@59
PS41, Line 59: size_t Count() {
> Can this function be marked const noexcept?
It acquires a lock, which is not const or noexcept.
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@65
PS41, Line 65: SYSTEM_TABLE = 8
> Add a brief comment
Done
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672
PS41, Line 672: QUERY_LIVE
> Add a brief comment
Done
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Hello Quanlong Huang, k.venureddy2...@gmail.com, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21019 to look at the new patch set (#9). Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor IMPALA-11534 skips reloading file metadata for some trivial ALTER_TABLE events. However, ALTER_TABLE events that have trivial changes in StorageDescriptor are not handled in IMPALA-11534. The only changes that require file metadata reload are: location, rowformat, fileformat, serde, and storedAsSubDirectories. The file metadata reload can be skipped for all other changes in SD. Testing: 1) Manual testing by changing SD parameters in local environment. 2) Added unit tests for the same in MetastoreEventsProcessorTest class. Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac --- M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 5 files changed, 172 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/21019/9 -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 9 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#10). Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. IMPALA-12872: Use Calcite for optimization - part 1: simple queries This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/set-classpath.sh M bin/start-impala-cluster.py M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/21019 )
Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE
events with trivial changes in StorageDescriptor
..
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21019/9/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:
http://gerrit.cloudera.org:8080/#/c/21019/9/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1904
PS9, Line 1904: } else if
(!isCustomTblPropsChanged(whitelistedTblProperties, beforeTable, afterTable)) {
line too long (95 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/21019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac
Gerrit-Change-Number: 21019
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sai Hemanth Gantasala
Gerrit-Comment-Date: Wed, 20 Mar 2024 23:50:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12916: Fix test event processor error global invalidate test random failure
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21169 ) Change subject: IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Gerrit-Change-Number: 21169 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Wed, 20 Mar 2024 23:42:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12916: Fix test event processor error global invalidate test random failure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21169 ) Change subject: IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Gerrit-Change-Number: 21169 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Wed, 20 Mar 2024 23:43:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12916: Fix test event processor error global invalidate test random failure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21169 ) Change subject: IMPALA-12916: Fix test_event_processor_error_global_invalidate test random failure .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10398/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I787cff4cc9f9df345cd715c02b51b8d93a150edf Gerrit-Change-Number: 21169 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Wed, 20 Mar 2024 23:43:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15592/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 9 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 23:03:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15591/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 8 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 22:59:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#9). Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. IMPALA-12872: Use Calcite for optimization - part 1: simple queries This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M bin/set-classpath.sh M bin/start-impala-cluster.py M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A
[Impala-ASF-CR] IMPALA-12699: Set timeout for catalog RPCs
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21146 ) Change subject: IMPALA-12699: Set timeout for catalog RPCs .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad39a79d0c89f2b04380f610a7e60558429e9c6e Gerrit-Change-Number: 21146 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Wed, 20 Mar 2024 22:33:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#8). Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. IMPALA-12872: Use Calcite for optimization - part 1: simple queries This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M bin/set-classpath.sh M bin/start-impala-cluster.py M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21173 ) Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. Patch Set 1: Code-Review+1 Looks OK. Tested it in my local machine and zooming stretch the timeline just to right direction instead of both left and right before this patch. -- To view, visit http://gerrit.cloudera.org:8080/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 22:32:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/21109/8/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/21109/8/bin/start-impala-cluster.py@629 PS8, Line 629: = flake8: E225 missing whitespace around operator http://gerrit.cloudera.org:8080/#/c/21109/8/tests/custom_cluster/test_calcite_planner.py File tests/custom_cluster/test_calcite_planner.py: http://gerrit.cloudera.org:8080/#/c/21109/8/tests/custom_cluster/test_calcite_planner.py@21 PS8, Line 21: import os flake8: F401 'os' imported but unused -- To view, visit http://gerrit.cloudera.org:8080/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 8 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 22:34:34 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21161/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21161/1//COMMIT_MSG@14 PS1, Line 14: : This patch builds both the shared and static library of GTest so Impala : can pick the corresponding library correctly. > nit: Please educate me, when shared vs static library is used? The shared libs are used when Impala is built with -so. Otherwise, the static libs are used. Here is the Impala patch for using this: https://gerrit.cloudera.org/c/21163/ Currently, the shared lib of gtest is missing. So Impala will link the static lib of gtest even when building with -so, i.e. all binary files including unifiedbetests and libkudu_test_util.so will statically link libgtest.a. https://github.com/apache/impala/blob/814acf3bbf015ca305603d33dada8f4233d929d0/CMakeLists.txt#L195-L198 A double-free memory issue like this would happen when using unifiedbetests: https://stackoverflow.com/a/23499556/5996453 -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 20 Mar 2024 22:28:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/21074/7/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/7/be/src/service/impala-server.cc@1439 PS7, Line 1439: static int32_t nit: can be static inline? -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 22:20:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12602: Unregister queries on idle timeout
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21074 ) Change subject: IMPALA-12602: Unregister queries on idle timeout .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/21074/7/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21074/7/be/src/service/impala-server.cc@2863 PS7, Line 2863: status nit: mark const http://gerrit.cloudera.org:8080/#/c/21074/7/docs/topics/impala_timeouts.xml File docs/topics/impala_timeouts.xml: http://gerrit.cloudera.org:8080/#/c/21074/7/docs/topics/impala_timeouts.xml@130 PS7, Line 130: exception details. Please add an explanation that the status will be INACTIVE_QUERY_EXPIRED if the query was in a good state at expiration or will be the error status if the query was not in a good state at expiration. -- To view, visit http://gerrit.cloudera.org:8080/21074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0 Gerrit-Change-Number: 21074 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Wed, 20 Mar 2024 21:54:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Jason Fehr has posted comments on this change. (
http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table
..
Patch Set 41:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h
File be/src/exec/system-table-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h@40
PS41, Line 40: by fetching more data from metrics.
Nit: this comment does not make sense in this context. Possibly a
copy-and-paste from somewhere else?
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h@43
PS41, Line 43: NYI
> Yeah, this pattern appears to be fairly common across PlanNode implementati
nit: please add a comment stating this function always returns an error
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.h
File be/src/exec/system-table-scanner.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.h@42
PS41, Line 42: bool eos() const { return eos_; }
nit: add noexcept?
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@509
PS41, Line 509: size_t NumLiveQueries();
Please mark this function const noexcept.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@1138
PS41, Line 1138: /// Returns a list of completed queries that have not yet
been written to storage.
Please document that this function makes a copy of the completed queries queue
and returns it so that callers know thery do not need to hold any locks before
calling this function.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/impala-server.h@1139
PS41, Line 1139: std::vector>
GetCompletedQueries();
Please mark this function const noexcept. The completed_queries_lock_ will
also need to be marked mutable.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/util/sharded-query-map-util.h
File be/src/util/sharded-query-map-util.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/util/sharded-query-map-util.h@59
PS41, Line 59: size_t Count() {
Can this function be marked const noexcept?
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift
File common/thrift/SystemTables.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift@21
PS41, Line 21: enum TQueryColumn {
> nit: I think TSystemTableColumn is a better fit.
Since this enum is for the active queries table which is a subclass of system
table, the name TQueryColumn is more descriptive since there could be multiple
subclasses of system table in the future.
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@52
PS41, Line 52: private static Map
SYSTEM_TABLE_NAME_MAP =
: ImmutableMap.of(QUERY_LIVE, TSystemTableName.QUERY_LIVE);
> Consider using ImmutableMap.
And assert map size is 1
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java
File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@121
PS41, Line 121: 1024L * 1024L
> Looks like 1MB is a standard MIN_MEMORY_ESTIMATE.
Could also use table_.getNumRows() multiplied by some constant to get a better
guess. I agree though that it's fine as-is and can be left unchanged.
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@36
PS41, Line 36: assert_query('sys.impala_query_live', self.client,
'test_query_live_1',
> Is there any race where the data about the query can get written before we
There is a thread that flushes the completed queries in-memory queue to the
sys.impala_query_log table, and that thread starts off sleeping for the
configured amount of time (default 5 minutes). Since queries that have
completed but are not yet flushed to disk show in the sys.impala_query_live
table, the queries will be in the sys.impala_query_live table at this point
unless the call to self.client.execute slept for more than 5 minutes after the
query closed.
--
To view, visit http://gerrit.cloudera.org:8080/20762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743
Gerrit-Change-Number: 20762
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21174 ) Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15590/ : 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/21174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 Gerrit-Change-Number: 21174 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 Mar 2024 21:19:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4545: Simplify test dimension in test decimal casting.py
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21174 Change subject: IMPALA-4545: Simplify test dimension in test_decimal_casting.py .. IMPALA-4545: Simplify test dimension in test_decimal_casting.py This patch split precision and scale as their own dimension and then constraint them to yield valid decimal type. Also done minor refactoring to reduce test skipping and pass flake8. After this patch, core exploration has 214 test items and exhaustive exploration has 12312 test items. Before, they were 408 and 12464 respectively. Testing: - Pass test_decimal_casting.py in core and exhaustive exploration. Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 --- M tests/query_test/test_decimal_casting.py 1 file changed, 49 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/21174/1 -- To view, visit http://gerrit.cloudera.org:8080/21174 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibe269e08a955097ad9e924d5d64b42438ad15be2 Gerrit-Change-Number: 21174 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto
[Impala-ASF-CR] IMPALA-11938: Raised error if NUM NODES is set to invalid value.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21147 ) Change subject: IMPALA-11938: Raised error if NUM_NODES is set to invalid value. .. IMPALA-11938: Raised error if NUM_NODES is set to invalid value. If NUM_NODES is set to a value other than 0 or 1 below error is raised ERROR: Invalid value for query option NUM_NODES: is not in range [0, 1] Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Reviewed-on: http://gerrit.cloudera.org:8080/21147 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M tests/hs2/test_hs2.py 3 files changed, 4 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Gerrit-Change-Number: 21147 Gerrit-PatchSet: 8 Gerrit-Owner: Anshula Jain Gerrit-Reviewer: Anshula Jain Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-11938: Raised error if NUM NODES is set to invalid value.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21147 ) Change subject: IMPALA-11938: Raised error if NUM_NODES is set to invalid value. .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Gerrit-Change-Number: 21147 Gerrit-PatchSet: 7 Gerrit-Owner: Anshula Jain Gerrit-Reviewer: Anshula Jain Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 20:42:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12626: Add Tables Queried to profile/history
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/20886 ) Change subject: IMPALA-12626: Add Tables Queried to profile/history .. Patch Set 25: (2 comments) I read through quickly http://gerrit.cloudera.org:8080/#/c/20886/25//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20886/25//COMMIT_MSG@10 PS25, Line 10: comma-separated list of tables accessed during a query: I think the table names are sorted? If so let's note it here. http://gerrit.cloudera.org:8080/#/c/20886/25/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/20886/25/common/thrift/Frontend.thrift@682 PS25, Line 682: 20: optional list tables Add a comment -- To view, visit http://gerrit.cloudera.org:8080/20886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c9c80b2adf7f3e44225a191fe8eb9df3c4bc5aa Gerrit-Change-Number: 20886 Gerrit-PatchSet: 25 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Wed, 20 Mar 2024 20:25:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table
..
Patch Set 41:
(19 comments)
All looks good, I have a few nits and questions
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@26
PS41, Line 26: #include "runtime/decimal-value.h"
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@27
PS41, Line 27: #include "runtime/decimal-value.inline.h"
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@71
PS41, Line 71: MaterializeNextRow
WriteStringSlot?
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@157
PS41, Line 157: for (int i = 0; i < tuple_desc->slots().size(); ++i) {
Nit: for (auto slot_desc : tuple_desc->slots())
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@65
PS41, Line 65: SYSTEM_TABLE = 8
Add a brief comment
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672
PS41, Line 672: QUERY_LIVE
> Well, I do use it to constrain the options in TSystemTable, and it reduces
Add a brief comment
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift
File common/thrift/SystemTables.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift@21
PS41, Line 21: enum TQueryColumn {
Add a note that this must be kept in sync with workload-management-fields.cc
(IIUIC)
(and possibly a back-link from there to here)
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/Db.java
File fe/src/main/java/org/apache/impala/catalog/Db.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/Db.java@134
PS41, Line 134: "sys"
Nit: should we have a constant for this?
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@52
PS41, Line 52: private static Map
SYSTEM_TABLE_NAME_MAP =
could be final?
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@184
PS41, Line 184: String, String
unnecessary?
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@186
PS41, Line 186: FieldSchema
unnecessary?
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java
File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@21
PS41, Line 21: import java.util.Map;
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@23
PS41, Line 23: import org.slf4j.Logger;
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@24
PS41, Line 24: import org.slf4j.LoggerFactory;
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@43
PS41, Line 43: import org.apache.impala.thrift.TSystemTableName;
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@46
PS41, Line 46: import com.google.common.collect.ImmutableMap;
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@30
PS41, Line 30: vector
unused
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@36
PS41, Line 36: assert_query('sys.impala_query_live', self.client,
'test_query_live_1',
Is there any race where the data about the query can get written before we look
at impala_query_live?
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@64
PS41, Line 64: vector
unused
--
To view, visit http://gerrit.cloudera.org:8080/20762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743
Gerrit-Change-Number: 20762
Gerrit-PatchSet: 41
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15589/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 20:13:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15588/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 6 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 20:13:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table
..
Patch Set 41:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/service/fe-support.cc@655
PS41, Line 655: for (const auto& it : membership_snapshot->current_backends) {
: if (it.second.has_is_coordinator() &&
it.second.is_coordinator()) {
: VLOG_QUERY << "Found coordinator "
: << it.second.address().hostname() << ":" <<
it.second.address().port();
:
coordinators.emplace_back(FromNetworkAddressPB(it.second.address()));
: }
: }
What do you think about pushing this to ClusterMembershipMgr or
ClusterMembershipMgr::Snapshot?
That will allow adding BE test in scheduler-test.cc / scheduler-test-util.cc
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672
PS41, Line 672: QUERY_LIVE
> This pattern gives flexibility to add more system tables in the future shou
Ack, In that case, please assign cardinal number.
QUERY_LIVE = 0
Also add comment on where to find the actual name string.
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/StatestoreService.thrift@207
PS41, Line 207: // A list of network addresses
: struct TAddressesList {
: 1: required list addresses;
: }
Move this to Types.thrift?
I understand this is required to serialize response for NativeGetCoordinators().
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift
File common/thrift/SystemTables.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift@21
PS41, Line 21: enum TQueryColumn {
nit: I think TSystemTableColumn is a better fit.
Please also put comment that these columns are all currently for QUERY_LIVE /
impala_query_live table.
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@52
PS41, Line 52: private static Map
SYSTEM_TABLE_NAME_MAP =
: ImmutableMap.of(QUERY_LIVE, TSystemTableName.QUERY_LIVE);
Consider using ImmutableMap.
Please also add FE test to assert the key-value pair here. Maybe something like
in FileSystemUtilTest.java.
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@176
PS41, Line 176:
: private static org.apache.hadoop.hive.metastore.api.Table
: createMetastoreTable(String dbName, String tableName,
String owner) {
> Right, it doesn't send anything to HMS. This is an Impala-only table, that
Ack
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java
File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@121
PS41, Line 121: 1024L * 1024L
> The completed queries table cuts off the SQL and PLAN fields based on coord
Looks like 1MB is a standard MIN_MEMORY_ESTIMATE.
https://github.com/apache/impala/blob/814acf3bbf015ca305603d33dada8f4233d929d0/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java#L148-L152
With that consideration, I'm okay with leaving this as is. We can refine later
if we found an issue.
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@27
PS41, Line 27:
@CustomClusterTestSuite.with_args(impalad_args="--enable_workload_mgmt "
:
"--cluster_id=test_query_live_1",
:
catalogd_args="--enable_workload_mgmt")
> sys.impala_query_live is currently only accessible if workload management i
Ack
--
To view, visit http://gerrit.cloudera.org:8080/20762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743
Gerrit-Change-Number: 20762
Gerrit-PatchSet: 41
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Andrew Sherman
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 Mar 2024 20:09:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 5: (8 comments) http://gerrit.cloudera.org:8080/#/c/21109/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21109/5//COMMIT_MSG@7 PS5, Line 7: IMPALA-12872: Use Calcite for ... > nit: combine the two lines. it seems short enough. How about 'Use Calcite Done http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java: http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java@33 PS5, Line 33: analyzer > nit:use underscore to be consistent Removed this since it is not needed. The analyzer is in the PlannerContext http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java@36 PS5, Line 36: ctx > nit: use underscore to be consistent with other variable names. Done http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java@39 PS5, Line 39: public final Class parentClass_; > We should use the actual base class name (RelNode/ImpalaPlanRel) instead of Removed this since it is not needed in this cut http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java: http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java@31 PS5, Line 31: tableMap > nit: use underscore Done http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java: http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@78 PS5, Line 78: private List foreignKeys; > This field is not used in this implementation. Is it for future use ? Removed it since not used in this cut http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java: http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@87 PS5, Line 87: LOG.debug(getDebugString(resultObject)); > nit: should this logging be in an else block ? Good catch I put a return in the if block. Also did this in other Calcite* files http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java: http://gerrit.cloudera.org:8080/#/c/21109/5/java/experimental-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java@33 PS5, Line 33: > nit: missing 'are' . Here and subsequent messages. Done -- To view, visit http://gerrit.cloudera.org:8080/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 19:50:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#7). Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. IMPALA-12872: Use Calcite for optimization - part 1: simple queries This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Hello Aman Sinha, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#6). Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. IMPALA-12872: Use Calcite for optimization - part 1: simple queries This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A
[Impala-ASF-CR] IMPALA-12872: Use Calcite for optimization - part 1: simple queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/21109/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java: http://gerrit.cloudera.org:8080/#/c/21109/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/NotSupported.java@39 PS6, Line 39: public static String APPX_COUNT_DISTINCT = "Approximate count distinct is not supported."; line too long (92 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 6 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Wed, 20 Mar 2024 19:47:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] PROTOTYPE: IMPALA-12905: Disk-based tuple caching
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21171 ) Change subject: PROTOTYPE: IMPALA-12905: Disk-based tuple caching .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15587/ : 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/21171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf Gerrit-Change-Number: 21171 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Wed, 20 Mar 2024 19:39:01 + Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-12905: Disk-based tuple caching
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21171 ) Change subject: PROTOTYPE: IMPALA-12905: Disk-based tuple caching .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10397/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf Gerrit-Change-Number: 21171 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Wed, 20 Mar 2024 19:24:24 + Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-12905: Disk-based tuple caching
Hello Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21171 to look at the new patch set (#3). Change subject: PROTOTYPE: IMPALA-12905: Disk-based tuple caching .. PROTOTYPE: IMPALA-12905: Disk-based tuple caching This implements storage for the tuple cache using local files. This contains contributions from Michael Smith, Yida Wu, and Joe McDonnell. TODO: Add more detail to this commit message Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf --- M be/src/exec/CMakeLists.txt M be/src/exec/tuple-cache-node.cc M be/src/exec/tuple-cache-node.h A be/src/exec/tuple-read-write-test.cc A be/src/exec/tuple-reader.cc A be/src/exec/tuple-reader.h A be/src/exec/tuple-writer.cc A be/src/exec/tuple-writer.h M be/src/runtime/CMakeLists.txt M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h A be/src/runtime/tuple-cache-mgr-test.cc A be/src/runtime/tuple-cache-mgr.cc A be/src/runtime/tuple-cache-mgr.h M bin/start-impala-cluster.py M common/thrift/metrics.json A tests/custom_cluster/test_tuple_cache.py 17 files changed, 1,757 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/21171/3 -- To view, visit http://gerrit.cloudera.org:8080/21171 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I13a65c4c0559cad3559d5f714a074dd06e9cc9bf Gerrit-Change-Number: 21171 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Query Live Table .. Patch Set 41: (3 comments) http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672 PS41, Line 672: QUERY_LIVE > Well, I do use it to constrain the options in TSystemTable, and it reduces This pattern gives flexibility to add more system tables in the future should the need arise. I also would prefer to keep the actual table name out of the Thrift definitions. http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java: http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@121 PS41, Line 121: 1024L * 1024L > No, honestly no idea what to put here. I guess this is another area we coul The completed queries table cuts off the SQL and PLAN fields based on coordinator startup flags (Default 16mb or 16,777,216 characters). We could do something similar here. Given that the SQL/PLAN/EXEC SUMMARY fields are less important in running queries since they can all be found on the Impala coordinator debug UI, I propose setting a hard, non-configurable limit that is much smaller than 16mb. http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@24 PS41, Line 24: class TestQueryLive(CustomClusterTestSuite): > Add test querying sys.impala_query_live over cluster with multiple coordina Since these tests do not change the size of the test clusters, they will all run against a 3-node cluster. It would be good though to submit queries to each coordinator and then verify that all three queries appear in the sys.impala_query_live table. -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 41 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: Wed, 20 Mar 2024 18:44:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12896 (Part 2): JDBC table must be created as external table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21159 ) Change subject: IMPALA-12896 (Part 2): JDBC table must be created as external table .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15586/ : 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/21159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5533b52434cdf1c430e30ac28a0146ab4d9d4b9 Gerrit-Change-Number: 21159 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Wed, 20 Mar 2024 18:25:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Query Live Table
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/20762 )
Change subject: IMPALA-12540: Query Live Table
..
Patch Set 41:
(5 comments)
I'll work on the other comments.
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h
File be/src/exec/system-table-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scan-node.h@43
PS41, Line 43: NYI
> nit: Not Yet Implemented?
Yeah, this pattern appears to be fairly common across PlanNode implementations.
See also data-source-scan-node.h and hbase-scan-node.h.
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/CatalogObjects.thrift@672
PS41, Line 672: QUERY_LIVE
> Since QUERY_LIVE name is constant, should this be a const instead?
Well, I do use it to constrain the options in TSystemTable, and it reduces data
needing to be serialized.
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@176
PS41, Line 176:
: private static org.apache.hadoop.hive.metastore.api.Table
: createMetastoreTable(String dbName, String tableName,
String owner) {
> This is not registering the table to HMS, isn't it?
Right, it doesn't send anything to HMS. This is an Impala-only table, that can
only be satisfied by coordinators, so not sure what anyone would do with it if
it were in HMS.
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java
File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java:
http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@121
PS41, Line 121: 1024L * 1024L
> SQL, PLAN, and EXEC_SUMMARY can be long. Has this take account for that?
No, honestly no idea what to put here. I guess this is another area we could
estimate from current active queries.
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:
http://gerrit.cloudera.org:8080/#/c/20762/41/tests/custom_cluster/test_query_live.py@27
PS41, Line 27:
@CustomClusterTestSuite.with_args(impalad_args="--enable_workload_mgmt "
:
"--cluster_id=test_query_live_1",
:
catalogd_args="--enable_workload_mgmt")
> Can this and the other test work without workload management enabled?
sys.impala_query_live is currently only accessible if workload management is
enabled. That's flagged in CatalogServiceCatalog.java,
CatalogBlacklistUtils.java, and Db.java.
--
To view, visit http://gerrit.cloudera.org:8080/20762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743
Gerrit-Change-Number: 20762
Gerrit-PatchSet: 41
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: Wed, 20 Mar 2024 18:08:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12896 (Part 2): JDBC table must be created as external table
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21159 ) Change subject: IMPALA-12896 (Part 2): JDBC table must be created as external table .. IMPALA-12896 (Part 2): JDBC table must be created as external table In some of the deployment environments, default table type is transactional. In these scenarios, JDBC tables which are created as non external table are not accepted by HMS due to strict managed table check failures. This patch forces JDBC tables to be created as external table, and requires at least 1 column for JDBC tables. Testing: - Updated frontend unit tests and end-to-end unit tests to create JDBC tables as external tables. - Passed core tests Change-Id: Ib5533b52434cdf1c430e30ac28a0146ab4d9d4b9 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java M testdata/bin/create-ext-data-source-table.sql M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables-predicates.test M testdata/workloads/functional-query/queries/QueryTest/impala-ext-jdbc-tables.test M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source-with-keystore.test M testdata/workloads/functional-query/queries/QueryTest/jdbc-data-source.test M testdata/workloads/functional-query/queries/QueryTest/mysql-ext-jdbc-tables.test 9 files changed, 37 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/21159/2 -- To view, visit http://gerrit.cloudera.org:8080/21159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib5533b52434cdf1c430e30ac28a0146ab4d9d4b9 Gerrit-Change-Number: 21159 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: gaurav singh
[Impala-ASF-CR] IMPALA-12626: Add Tables Queried to profile/history
Jason Fehr has posted comments on this change. (
http://gerrit.cloudera.org:8080/20886 )
Change subject: IMPALA-12626: Add Tables Queried to profile/history
..
Patch Set 25: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20886/25/be/src/util/debug-util.cc
File be/src/util/debug-util.cc:
http://gerrit.cloudera.org:8080/#/c/20886/25/be/src/util/debug-util.cc@287
PS25, Line 287: }
Nit: this may be cleaner if re-written using a pattern I have seen elsewhere in
your code:
for (auto const& tbl : tbls) {
ss << tbl->db_name << "." << tbl->table_name << ",";
}
return ss.str().pop_back();
--
To view, visit http://gerrit.cloudera.org:8080/20886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c9c80b2adf7f3e44225a191fe8eb9df3c4bc5aa
Gerrit-Change-Number: 20886
Gerrit-PatchSet: 25
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Wed, 20 Mar 2024 17:55:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21173 ) Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15585/ : 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/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 Mar 2024 16:54:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12362: (part-1/4) Refactor service management scripts.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20921 ) Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts. .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15584/ : 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/20921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 Gerrit-Change-Number: 20921 Gerrit-PatchSet: 14 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Wed, 20 Mar 2024 16:33:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12362: (part-1/4) Refactor service management scripts.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20921 ) Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts. .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15583/ : 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/20921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 Gerrit-Change-Number: 20921 Gerrit-PatchSet: 13 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Wed, 20 Mar 2024 16:32:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline
Surya Hebbar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21173 Change subject: IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline .. IMPALA-12923: Fix header alignment during horizontal scrolling in query timeline On horizontally scrolling the query timeline after zooming, the controls and navigation bar at the top of the page move away from the visibility area. This makes it difficult to use the controls while scrolling. The controls and navigation bar now stay within the visibility area even after scrolling horizontally, allowing for easier navigation. Hovering on the edges to scroll horizontally is also supported as before. Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e --- M www/query_timeline.tmpl M www/scripts/query_timeline/fragment_diagram.js M www/scripts/query_timeline/fragment_metrics_diagram.js M www/scripts/query_timeline/host_utilization_diagram.js 4 files changed, 35 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/21173/1 -- To view, visit http://gerrit.cloudera.org:8080/21173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib3b248554ba32766c29faf91c791f2dbbd20641e Gerrit-Change-Number: 21173 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar
[Impala-ASF-CR] IMPALA-12362: (part-1/4) Refactor service management scripts.
Xiang Yang has posted comments on this change. (
http://gerrit.cloudera.org:8080/20921 )
Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts.
..
Patch Set 14:
(2 comments)
> Patch Set 12:
>
> (2 comments)
>
> It looks good, just a minor issue remains.
Hi zihao, thanks for pointing these out. I've finished the stop waiting step,
at the same time I remove the 'async mode' because it can be replaced by set
waiting counts to 0.
http://gerrit.cloudera.org:8080/#/c/20921/12/package/bin/impala.sh
File package/bin/impala.sh:
http://gerrit.cloudera.org:8080/#/c/20921/12/package/bin/impala.sh@93
PS12, Line 93: *) usage && exit 1 ;;
> Shouldn't we execute the kill command before this?
remove the 'async mode'.
http://gerrit.cloudera.org:8080/#/c/20921/12/package/bin/impala.sh@95
PS12, Line 95: done
: check_counts ${counts} ${period}
: local service_pidfile_key=${service^^}_PIDFILE
: local service_pidfile=${!service_pidfile_key}
: if [[ ! -f ${service_pidfile} ]]; then
: echo "Already stopped: PID file '${service_pidfile}' not
found."
: exit 0
: fi
: local pid=$(cat ${service_pidfile})
: if ! ps -p ${pid} -o comm=|grep ${service} &> /dev/null ; then
> nit: I think it would be better to separate this piece of logic into a func
Done
--
To view, visit http://gerrit.cloudera.org:8080/20921
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147
Gerrit-Change-Number: 20921
Gerrit-PatchSet: 14
Gerrit-Owner: Xiang Yang
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Xiang Yang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Wed, 20 Mar 2024 16:15:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12362: (part-1/4) Refactor service management scripts.
Hello Quanlong Huang, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20921 to look at the new patch set (#14). Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts. .. IMPALA-12362: (part-1/4) Refactor service management scripts. Uniform all service management scripts to 'bin/impala.sh', administrator can customize environment variables based on their cluster at 'conf/impala-env.sh', as well as set flags at 'conf/impalad_flags...'. Usually administrator can override the environment variables in 'conf/impala-env.sh' with commandline arguments. The same is true for flags in 'conf/impalad_flags...'. This flexibility can be used in scenarios such as supporting multi-instance deployments. The directory structure has been adjusted as follows: - put java libs to 'lib/jars' directory. - put native libs to 'lib/native' directory. - put impalad binary to 'sbin' directory. Testing: - Manually deploy packages on Ubuntu22.04 and verify it. Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 --- M CMakeLists.txt M be/src/service/CMakeLists.txt D package/bin/impala-env.sh A package/bin/impala.sh D package/bin/start-catalogd.sh D package/bin/start-impalad.sh D package/bin/start-statestored.sh D package/bin/stop-catalogd.sh D package/bin/stop-impalad.sh D package/bin/stop-statestored.sh M package/conf/catalogd_flags A package/conf/impala-env.sh M package/conf/impalad_flags R package/conf/statestored_flags 14 files changed, 324 insertions(+), 290 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20921/14 -- To view, visit http://gerrit.cloudera.org:8080/20921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 Gerrit-Change-Number: 20921 Gerrit-PatchSet: 14 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12362: (part-1/4) Refactor service management scripts.
Hello Quanlong Huang, Zihao Ye, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20921 to look at the new patch set (#13). Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts. .. IMPALA-12362: (part-1/4) Refactor service management scripts. Uniform all service management scripts to 'bin/impala.sh', administrator can customize environment variables based on their cluster at 'conf/impala-env.sh', as well as set flags at 'conf/impalad_flags...'. Usually administrator can override the environment variables in 'conf/impala-env.sh' with commandline arguments. The same is true for flags in 'conf/impalad_flags...'. This flexibility can be used in scenarios such as supporting multi-instance deployments. The directory structure has been adjusted as follows: - put java libs to 'lib/jars' directory. - put native libs to 'lib/native' directory. - put impalad binary to 'sbin' directory. Testing: - Manually deploy packages on Ubuntu22.04 and verify it. Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 --- M CMakeLists.txt M be/src/service/CMakeLists.txt D package/bin/impala-env.sh A package/bin/impala.sh D package/bin/start-catalogd.sh D package/bin/start-impalad.sh D package/bin/start-statestored.sh D package/bin/stop-catalogd.sh D package/bin/stop-impalad.sh D package/bin/stop-statestored.sh M package/conf/catalogd_flags A package/conf/impala-env.sh M package/conf/impalad_flags R package/conf/statestored_flags 14 files changed, 323 insertions(+), 290 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20921/13 -- To view, visit http://gerrit.cloudera.org:8080/20921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147 Gerrit-Change-Number: 20921 Gerrit-PatchSet: 13 Gerrit-Owner: Xiang Yang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xiang Yang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11938: Raised error if NUM NODES is set to invalid value.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21147 ) Change subject: IMPALA-11938: Raised error if NUM_NODES is set to invalid value. .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Gerrit-Change-Number: 21147 Gerrit-PatchSet: 7 Gerrit-Owner: Anshula Jain Gerrit-Reviewer: Anshula Jain Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 15:45:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11938: Raised error if NUM NODES is set to invalid value.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21147 ) Change subject: IMPALA-11938: Raised error if NUM_NODES is set to invalid value. .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10396/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Gerrit-Change-Number: 21147 Gerrit-PatchSet: 7 Gerrit-Owner: Anshula Jain Gerrit-Reviewer: Anshula Jain Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 15:45:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11938: Raised error if NUM NODES is set to invalid value.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21147 ) Change subject: IMPALA-11938: Raised error if NUM_NODES is set to invalid value. .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21147 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib283ae350875a7ac217490c216a26281f1bb4812 Gerrit-Change-Number: 21147 Gerrit-PatchSet: 6 Gerrit-Owner: Anshula Jain Gerrit-Reviewer: Anshula Jain Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 20 Mar 2024 15:44:00 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12915: Build shared library of GTest
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21161 ) Change subject: IMPALA-12915: Build shared library of GTest .. Patch Set 1: Code-Review+1 (1 comment) I think this is good, but somebody with more experience in build infrastructure code should weigh in. http://gerrit.cloudera.org:8080/#/c/21161/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21161/1//COMMIT_MSG@14 PS1, Line 14: : This patch builds both the shared and static library of GTest so Impala : can pick the corresponding library correctly. nit: Please educate me, when shared vs static library is used? -- To view, visit http://gerrit.cloudera.org:8080/21161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34023fe4fb0691fbca1dc547d1e40ada1beca22 Gerrit-Change-Number: 21161 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 20 Mar 2024 15:40:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15582/ : 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/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 Mar 2024 15:28:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
k.venureddy2...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21172 Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive CI failed for Ozone and S3 builds due to 5 tests failure in TestEventProcessingError. Those tests run insert into table and analyze table compute statistics queries on hive and failed to start the Tez client. Those tests were introduced with IMPALA-12832. Testing: - Executed end to end tests Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 --- M tests/custom_cluster/test_event_processing_error.py 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/21172/1 -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[Impala-ASF-CR] IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21172 ) Change subject: IMPALA-12917: Skip TestEventProcessingError tests requiring tez execution on hive .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10395/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb8422fbb494cd74def5ff6926aea82d0981cc82 Gerrit-Change-Number: 21172 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 Mar 2024 15:08:46 + Gerrit-HasComments: No
