[Impala-ASF-CR] IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14629 ) Change subject: IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py .. IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py If someone passes --skip-stress multiple times to tests/run-tests.py, it currently only removes one of the occurrences from the arguments and allows the other one to pass through to pytest. This causes pytest to immediately error out. This behavior is seen on the docker-based tests, because test-with-docker.py specifies --skip-stress and bin/run-all-tests.sh adds another --skip-stress for core runs. This changes tests/run-tests.py to handle multiple occurrences of --skip-stress, --skip-parallel, and --skip-serial. Testing: - Tested manually with duplicate skip flags. Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Reviewed-on: http://gerrit.cloudera.org:8080/14629 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M tests/run-tests.py 1 file changed, 14 insertions(+), 9 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Gerrit-Change-Number: 14629 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 9: Code-Review+2 > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5273/ The job failed due to "Script create-load-data.sh timed out". It probably is some jenkins issue. Will retrigger it one more time to make sure. -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 22 Nov 2019 06:35:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5278/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 22 Nov 2019 06:33:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5115/ : 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/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 22 Nov 2019 06:20:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14755 ) Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5271/ -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 22 Nov 2019 06:05:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Jiawei Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 9: rebase the commit to solve merge conflict. -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 22 Nov 2019 05:37:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Hello Quanlong Huang, Yongzhi Chen, Vihang Karajgaonkar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14611 to look at the new patch set (#9). Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. IMPALA-9110: Add table loading time break-down metrics for HdfsTable A. Problem: Catalog table loading currently only records the total loading time. We will need some break-down times, i.e. more detailed time recording on each loading function. Also, the table schema loading is not taken into account for load-duration. We will need to add some more metrics for that. B. Solution: - We added "hms-load-tbl-schema", "load-duration.all-column-stats", "load-duration.all-partitions.total-time", "load-duration.all-partitions.file-metadata". Also, we logged the loadValidWriteIdList() time. So now we have a more detailed breakdown time for table loading info. The table loading time metrics for HDFS tables are in the following hierarchy: - Table Schema Loading - Table Metadata Loading - total time - all column stats loading time - ValidWriteIds loading time - all partitions loading time - total time - file metadata loading time - storage-metadata-loading-time(standalone metric) 1. Table Schema Loading: * Meaning: The time for HMS to fetch table object and the real schema loading time. Normally, the code path is "msClient.getHiveClient().getTable(dbName, tblName)" * Metric : hms-load-tbl-schema 2. Table Metadata Loading -- total time * Meaning: The time to load all the table metadata. The code path is load() function in HdfsTable.load() function. * Metric: load-duration.total-time 2.1 Table Metadata Loading -- all column stats * Meaning: load all column stats, this is part of table metadata loading The code path is HdfsTable.loadAllColumnStats() * Metric: load-duration.all-column-stats 2.2 Table Metadata Loading -- loadValidWriteIdList * Meaning: fetch ValidWriteIds from HMS The code path is HdfsTable.loadValidWriteIdList() * Metric: no metric recorded for this one. Instead, a debug log is generated. 2.3 Table Metadata Loading -- storage metadata loading(standalone metric) * Meaning: Storage related to file system operations during metadata loading.(The amount of time spent loading metadata from the underlying storage layer.) * Metric: we rename it to load-duration.storage-metadata. This is a metric introduced by IMPALA-7322 2.4 Table Metadata Loading -- load all partitions * Meaning: Load all partitions time, including fetching all partitions from HMS and loading all partitions. The code path is MetaStoreUtil.fetchAllPartitions() and HdfsTable.loadAllPartitions() * Metric: load-duration.all-partitions 2.4.1 Table Metadata Loading -- load all partitions -- load file metadata * Meaning: The file metadata loading for all all partitions. (This is part of 2.4). Code path: loadFileMetadataForPartitions() inside loadAllPartitions() * Metric: load-duration.all-partitions.file-metadata C. Extra thing in this commit: 1. Add PrintUtils.printTimeNs for PrettyPrint time in FrontEnd 2. Add explanation for table loading manager D. Test: 1. Add Unit tests for PrintUtils.printTime() function 2. Manual describe table and verify the table loading metrics are correct. Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/common/PrintUtils.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java 10 files changed, 186 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/14611/9 -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 9 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-9107 (part 1): Add scripts to produce an m2 archive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14562 ) Change subject: IMPALA-9107 (part 1): Add scripts to produce an m2 archive .. Patch Set 12: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5275/ -- To view, visit http://gerrit.cloudera.org:8080/14562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I043912f5fbc7cf24ee80b2855354656aa587ca9f Gerrit-Change-Number: 14562 Gerrit-PatchSet: 12 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 05:12:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC"
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14780 ) Change subject: IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5277/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32371a515fb607da396914502da8c7fb071406bc Gerrit-Change-Number: 14780 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 05:08:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14629 ) Change subject: IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Gerrit-Change-Number: 14629 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Fri, 22 Nov 2019 05:08:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 ) Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5114/ : 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/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 04:21:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8065: Add OS distribution name in OSInfo
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14531 ) Change subject: IMPALA-8065: Add OS distribution name in OSInfo .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5113/ : 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/14531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a Gerrit-Change-Number: 14531 Gerrit-PatchSet: 9 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Fri, 22 Nov 2019 04:04:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC"
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14780 ) Change subject: IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5276/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32371a515fb607da396914502da8c7fb071406bc Gerrit-Change-Number: 14780 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 03:40:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 ) Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG@27 PS2, Line 27: illegal state tran > the (case->if) statement in UpdateNonErrorOperationState would ensure that Good point. Updated the commit message. wrt UpdateNonErrorOperationState, it seems the current behavior is to ignore state transitions that are invalid, I think it would be cleaner to enforce the transitions using DCHECKs and fix any attempts to perform an illegal state transition. this can help surface bugs in the code if the an illegal state transition is ever attempted. http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@a278 PS2, Line 278: : : > i think the reason we chose to do this here is because we wanted to guarant changed it so that if there is a async_exec_thread_ thread, it sets the state to PENDING before the async_exec_thread_ is created. this way when the Exec RPC returns, the state should always be PENDING. http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@780 PS2, Line 780: result_metadata_ = metadata_op_result.schema > I think with a metadata op it is expected to transition directly to FINISHE Yeah, I made this change because thats what all the other operations do, even the ones that don't run asynchronously. So I think it makes sense to keep this consistent with the rest of the operations. Plus, it makes the state machine easier to reason about if everything transitions from INITIALIZED -> [PENDING] -> RUNNING -> FINISHED. http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@929 PS2, Line 929: $0 -> $1"; > nit: how about: Done -- To view, visit http://gerrit.cloudera.org:8080/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 03:37:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14744 to look at the new patch set (#3). Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. IMPALA-6894: Use an internal representation of query states in ClientRequestState Re-factors the state machine of ClientRequestState so that it uses an internal state represetation rather than using the one defined by TOperationState. The possible states are defined in ClientRequestState::ExecState and the possible state transitions are outlined in client-request-state.h and enforced in ClientRequestState::UpdateNonErrorExecState. The states defined in ClientRequestState::ExecState are the same states currently used in TOperationState. This patch simply makes it easy to define new states in the future. The value of ClientRequestState::ExecState is exposed to clients via the entry "Impala Query State" in the runtime profile. It is meant to be the Impala specific version of "Query State" (which is the Beeswax state). This allows Impala to expose its internal state without breaking existing clients that might rely on the value of "Query State". Additional Bug Fixes: * Previously, UpdateNonErrorOperationState would ignore attempts to make illegal state transitions, now the method uses DCHECKs to ensure only valid state transitions are attempted; this required fixing a possible race condition where a query could transition from RUNNING to PENDING * The ClientRequestState state is now tracked using an AtomicEnum, which fixes a few possible race conditions where the state was being read without holding the ClientRequestState::lock_ Testing: * Ran core tests * Added test to make sure "Impala Query State" is populated Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M tests/query_test/test_observability.py M tests/util/cancel_util.py 8 files changed, 190 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/3 -- To view, visit http://gerrit.cloudera.org:8080/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8065: Add OS distribution name in OSInfo
Xiaomeng Zhang has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/14531 ) Change subject: IMPALA-8065: Add OS distribution name in OSInfo .. IMPALA-8065: Add OS distribution name in OSInfo Before this change OsInfo::DebugString() would print two lines: - OS version: the long name of the Linux kernel from /proc/version - Clock: the type of clock used After this change OsInfo::DebugString() will print three lines: - OS distribution: the short name of the OS release. If Docker is being used this is the name of the Container OS. - OS version: the long name of the Linux kernel from /proc/version. If Docker is being used this is the description of the Host Kernel. - Clock: the type of clock used. Tested locally, the displayed OS Info in Ubuntu16 dev box is: OS distribution: Ubuntu 16.04.6 LTS OS version: Linux version 4.15.0-65-generic (buildd@lcy01-amd64-017) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.10)) Clock: clocksource: 'tsc', clockid_t: CLOCK_MONOTONIC Also checked with diff OS in docker: centos, redhat, ubuntu, oracle, debian to make sure /etc/os-release exists and PRETTY_NAME in that file. Each OS picked one version to test. Specially for centos6 and redhat6, which have redhat-release instead of os-release, copied redhat-release into Ubuntu16 dev box and verified os version in mini-cluster. Added new backend test os-info-test.cc. Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a --- M be/src/util/CMakeLists.txt A be/src/util/os-info-test.cc M be/src/util/os-info.cc M be/src/util/os-info.h 4 files changed, 82 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14531/9 -- To view, visit http://gerrit.cloudera.org:8080/14531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I848c9e53ee4e0bf8ae0874bb6da28e8efa7f7c8a Gerrit-Change-Number: 14531 Gerrit-PatchSet: 9 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xiaomeng Zhang
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 8: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5273/ -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 22 Nov 2019 02:16:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC"
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14780 ) Change subject: IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5112/ : 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/14780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32371a515fb607da396914502da8c7fb071406bc Gerrit-Change-Number: 14780 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 01:59:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 ) Change subject: IMPALA-9127: explicit probe state machine in hash join .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5111/ : 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/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 01:57:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8607: [DOCS] Document the new global level .impalarc file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14779 ) Change subject: IMPALA-8607: [DOCS] Document the new global level .impalarc file .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/536/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90195efceca8a03403f9a674f689648a04ab495d Gerrit-Change-Number: 14779 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Ethan Xue Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 22 Nov 2019 01:55:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8867: Further deflake test auto scaling
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14740 ) Change subject: IMPALA-8867: Further deflake test_auto_scaling .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5272/ -- To view, visit http://gerrit.cloudera.org:8080/14740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29808982cc6226152c544cb99f76961b582975a7 Gerrit-Change-Number: 14740 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Fri, 22 Nov 2019 01:43:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 ) Change subject: IMPALA-9127: explicit probe state machine in hash join .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5110/ : 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/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 01:23:30 + Gerrit-HasComments: No
[Impala-ASF-CR] Allow running backend tests sharded and in parallel
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13290 ) Change subject: Allow running backend tests sharded and in parallel .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5109/ : 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/13290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d44475e3f5a45c806f00d5003eeadf59683b009 Gerrit-Change-Number: 13290 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Nov 2019 01:08:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC"
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14780 ) Change subject: IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32371a515fb607da396914502da8c7fb071406bc Gerrit-Change-Number: 14780 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Fri, 22 Nov 2019 01:06:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hash join probe structures in build .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5108/ : 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/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 01:01:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC"
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14780 Change subject: IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" .. IMPALA-9154: Revert "IMPALA-7984: Port runtime filter from Thrift RPC to KRPC" The previous patch porting runtime filter from Thrift RPC to KRPC introduces a deadlock if there are a very limited number of threads on the Impala cluster. Specifically, in that patch a Coordinator used a synchronous KRPC to propagate an aggregated filter to other hosts. A deadlock would happen if there is no thread available on the receiving side to answer that KRPC especially the calling and receiving threads are called from the same thread pool. One possible way to address this issue is to make the call of propagating a runtime filter asynchronous to free the calling thread. Before resolving this issue, we revert this patch for now. This reverts commit ec11c18884988e838a8838e1e8ecc37461e1a138. Change-Id: I32371a515fb607da396914502da8c7fb071406bc --- M be/src/benchmarks/bloom-filter-benchmark.cc M be/src/runtime/backend-client.h M be/src/runtime/client-cache.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/data-stream-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h M be/src/runtime/exec-env.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.h M be/src/runtime/timestamp-value.h M be/src/scheduling/request-pool-service.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/frontend.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/bloom-filter-test.cc M be/src/util/bloom-filter.cc M be/src/util/bloom-filter.h M be/src/util/min-max-filter-test.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/protobuf/common.proto M common/protobuf/data_stream_service.proto M common/thrift/ImpalaInternalService.thrift 39 files changed, 754 insertions(+), 1,091 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/14780/1 -- To view, visit http://gerrit.cloudera.org:8080/14780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I32371a515fb607da396914502da8c7fb071406bc Gerrit-Change-Number: 14780 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14688 to look at the new patch set (#8). Change subject: IMPALA-9127: explicit probe state machine in hash join .. IMPALA-9127: explicit probe state machine in hash join This refactors the main loop in PartitionedHashJoinNode::GetNext() to use an explicit state machine, rather than the hard-to-follow implicit state machine previously used. A new state variable 'probe_state_' is used to drive the loop, with DCHECKs enforcing invariants of other member variables. I deliberately tried to minimise changes to other functions (including any attempts to factor logic out of GetNext()) to minimise the scope of this patch. The new logic is mostly equivalent to the old logic, although there may be a different number of trips through the loop because of the way the cascading checks in the old version worked. A few notable changes: * DoneProbing() is consistently called when probing is finished, including in cases, like probing a single spilled partition, where it wasn't previously. * The repeated AtCapacity() checks are consolidated into a single check that happens at the end of the loop. Resources attached to batches should still be flushed at the appropriate points, since each previous "if (out_batch->AtCapacity()) break;" corresponds to a new loop iteration in the new code. * OutputNullAwareNullProbe() and OutputNullAwareProbeRows() now explicitly signal when they are done using an output argument, instead of implicitly via AtCapacity(), which is incredibly error-prone. Testing: We have adequate coverage for different join modes, including with spilling. * Ran exhaustive tests. * Ran a single node stress test with TPC-H and TPC-DS * Ran a single node stress test with larger scale factor Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 --- M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 3 files changed, 352 insertions(+), 201 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/14688/8 -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 ) Change subject: IMPALA-9127: explicit probe state machine in hash join .. Patch Set 8: Code-Review+1 PS7 was the wrong version, sorry about that -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:46:18 + Gerrit-HasComments: No
[Impala-ASF-CR] Allow running backend tests sharded and in parallel
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/13290 ) Change subject: Allow running backend tests sharded and in parallel .. Patch Set 3: (1 comment) I refreshed this to take into account IMPALA-8170 converting a bunch of tests to the unified executable. Even when posting this, I know it has some comments that are out of date, so I think this needs a bit more work before its ready to merge. http://gerrit.cloudera.org:8080/#/c/13290/2/bin/run-backend-tests.sh File bin/run-backend-tests.sh: http://gerrit.cloudera.org:8080/#/c/13290/2/bin/run-backend-tests.sh@31 PS2, Line 31: = > `NUM_PROC`. Otherwise breaks as in https://jenkins.impala.io/job/ubuntu-16. Fixed -- To view, visit http://gerrit.cloudera.org:8080/13290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d44475e3f5a45c806f00d5003eeadf59683b009 Gerrit-Change-Number: 13290 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Nov 2019 00:39:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 ) Change subject: IMPALA-9127: explicit probe state machine in hash join .. Patch Set 7: Code-Review+1 Rebased, carry +1 -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:38:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8607: [DOCS] Document the new global level .impalarc file
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14779 ) Change subject: IMPALA-8607: [DOCS] Document the new global level .impalarc file .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/536/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90195efceca8a03403f9a674f689648a04ab495d Gerrit-Change-Number: 14779 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 22 Nov 2019 00:39:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8607: [DOCS] Document the new global level .impalarc file
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14779 Change subject: IMPALA-8607: [DOCS] Document the new global level .impalarc file .. IMPALA-8607: [DOCS] Document the new global level .impalarc file Change-Id: I90195efceca8a03403f9a674f689648a04ab495d --- M docs/topics/impala_shell_options.xml 1 file changed, 75 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/14779/1 -- To view, visit http://gerrit.cloudera.org:8080/14779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I90195efceca8a03403f9a674f689648a04ab495d Gerrit-Change-Number: 14779 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14688 to look at the new patch set (#7). Change subject: IMPALA-9127: explicit probe state machine in hash join .. IMPALA-9127: explicit probe state machine in hash join This refactors the main loop in PartitionedHashJoinNode::GetNext() to use an explicit state machine, rather than the hard-to-follow implicit state machine previously used. A new state variable 'probe_state_' is used to drive the loop, with DCHECKs enforcing invariants of other member variables. I deliberately tried to minimise changes to other functions (including any attempts to factor logic out of GetNext()) to minimise the scope of this patch. The new logic is mostly equivalent to the old logic, although there may be a different number of trips through the loop because of the way the cascading checks in the old version worked. A few notable changes: * DoneProbing() is consistently called when probing is finished, including in cases, like probing a single spilled partition, where it wasn't previously. * The repeated AtCapacity() checks are consolidated into a single check that happens at the end of the loop. Resources attached to batches should still be flushed at the appropriate points, since each previous "if (out_batch->AtCapacity()) break;" corresponds to a new loop iteration in the new code. * OutputNullAwareNullProbe() and OutputNullAwareProbeRows() now explicitly signal when they are done using an output argument, instead of implicitly via AtCapacity(), which is incredibly error-prone. Testing: We have adequate coverage for different join modes, including with spilling. * Ran exhaustive tests. * Ran a single node stress test with TPC-H and TPC-DS * Ran a single node stress test with a debug action to force spilling: --impalad_args="-default_query_options=DEBUG_ACTION=-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@0.5" Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 --- M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 3 files changed, 327 insertions(+), 200 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/14688/7 -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Allow running backend tests sharded and in parallel
Joe McDonnell has uploaded a new patch set (#3) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/13290 ) Change subject: Allow running backend tests sharded and in parallel .. Allow running backend tests sharded and in parallel This improves the CMake functions for adding a test, adding a NUM_SHARDS parameter and management of the RUN_SERIAL propery. See the newly added documentation for the ADD_BE_TEST() cmake function for details. This patch enables the new functionality for 'expr-test' which is one of the slowest backend tests. To test this, I enabled parallel runs of expr-test and tested that 'ctest -j 8 -R expr-test' runs the shards in parallel, and that different subsets of test cases run in each shard. This command executed in 230 secs vs 871secs serially (so should save about 10 minutes of test time) Along the way I also split up the longest-running test case in expr-test so that it would shard more easily. I also implemented the same functionality for unified tests and used it in a few of the longer-running ones which I could verify were single-threaded and shouldn't have any cross-test conflicts. Change-Id: I0d44475e3f5a45c806f00d5003eeadf59683b009 --- M be/CMakeLists.txt M be/src/catalog/CMakeLists.txt M be/src/codegen/CMakeLists.txt M be/src/common/CMakeLists.txt M be/src/exec/CMakeLists.txt M be/src/exec/parquet/CMakeLists.txt M be/src/experiments/CMakeLists.txt M be/src/exprs/CMakeLists.txt M be/src/exprs/expr-test.cc M be/src/rpc/CMakeLists.txt M be/src/runtime/CMakeLists.txt M be/src/runtime/bufferpool/CMakeLists.txt M be/src/runtime/io/CMakeLists.txt M be/src/scheduling/CMakeLists.txt M be/src/service/CMakeLists.txt M be/src/statestore/CMakeLists.txt M be/src/util/CMakeLists.txt M bin/run-backend-tests.sh 18 files changed, 261 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/13290/3 -- To view, visit http://gerrit.cloudera.org:8080/13290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0d44475e3f5a45c806f00d5003eeadf59683b009 Gerrit-Change-Number: 13290 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/14756 ) Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot .. Patch Set 1: Code-Review+2 (5 comments) LGTM I have a few nits http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@18 PS1, Line 18: join vs a PHJ. Maybe spell out "partitioned hash join" (if that is what this is)? Update: I see this in a few places, as a newbie I like seeing things spelled out. http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@21 PS1, Line 21: planner will use a configurable default which should be set to the Maybe mention that the default is 20? http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc@253 PS1, Line 253: DEFINE_int32(num_expected_executors, 20, "The number of executors that the planner " Maybe 'the number of executors that are expected to be available for the execution of a single query' http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java: http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@39 PS1, Line 39: // The set of hosts that are members of the cluster given by hostname. Should we add some caveats here (and for ipAddresses_) that these values are only present if we are not using executor groups (IIUC) http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java File fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java: http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java@83 PS1, Line 83: // Adding two or more executors will make the planner switch to a PHJ. partitioned hash join -- To view, visit http://gerrit.cloudera.org:8080/14756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9 Gerrit-Change-Number: 14756 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 22 Nov 2019 00:34:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9181: Serialize TQueryCtx once per query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14777 ) Change subject: IMPALA-9181: Serialize TQueryCtx once per query .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5107/ : 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/14777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6 Gerrit-Change-Number: 14777 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Fri, 22 Nov 2019 00:28:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9107 (part 1): Add scripts to produce an m2 archive
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14562 ) Change subject: IMPALA-9107 (part 1): Add scripts to produce an m2 archive .. Patch Set 12: Code-Review+2 Rebased, carry +2 -- To view, visit http://gerrit.cloudera.org:8080/14562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I043912f5fbc7cf24ee80b2855354656aa587ca9f Gerrit-Change-Number: 14562 Gerrit-PatchSet: 12 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:27:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9107 (part 1): Add scripts to produce an m2 archive
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14562 ) Change subject: IMPALA-9107 (part 1): Add scripts to produce an m2 archive .. Patch Set 12: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5275/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14562 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I043912f5fbc7cf24ee80b2855354656aa587ca9f Gerrit-Change-Number: 14562 Gerrit-PatchSet: 12 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:27:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14629 ) Change subject: IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py .. Patch Set 3: Code-Review+2 Rebased, carry +2 -- To view, visit http://gerrit.cloudera.org:8080/14629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Gerrit-Change-Number: 14629 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Fri, 22 Nov 2019 00:26:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14629 ) Change subject: IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5274/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Gerrit-Change-Number: 14629 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Fri, 22 Nov 2019 00:26:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#10). Change subject: IMPALA-9126: part 2: no hash join probe structures in build .. IMPALA-9126: part 2: no hash join probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 185 insertions(+), 182 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/10 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hash join probe structures in build .. Patch Set 9: Code-Review+1 carry +1 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:15:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14776 ) Change subject: IMPALA-7550: Add documentation to profile counters .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5106/ : 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/14776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 Gerrit-Change-Number: 14776 Gerrit-PatchSet: 1 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 22 Nov 2019 00:09:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9177: Bump min-query-mem-limit for dockerised tests
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14759 ) Change subject: IMPALA-9177: Bump min-query-mem-limit for dockerised tests .. IMPALA-9177: Bump min-query-mem-limit for dockerised tests A couple upstream dockerised test runs have seen TestTpchQuery.test_tpch query 18 on Kudu hit a memory limit. This is memory limit comes from the admission control settings, as admission control is enabled for the dockerised tests in IMPALA-8451. This bumps the min-query-mem-limit from 256MB to 300MB to avoid hitting this memory limit. Testing: - Ran GVO. Dockerised tests did not change in run length. Change-Id: I7f478b988a7bc2e7bd7422ceda52c6eddac04d0a Reviewed-on: http://gerrit.cloudera.org:8080/14759 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M fe/src/test/resources/minicluster-llama-site.xml 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7f478b988a7bc2e7bd7422ceda52c6eddac04d0a Gerrit-Change-Number: 14759 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9177: Bump min-query-mem-limit for dockerised tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14759 ) Change subject: IMPALA-9177: Bump min-query-mem-limit for dockerised tests .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f478b988a7bc2e7bd7422ceda52c6eddac04d0a Gerrit-Change-Number: 14759 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 23:56:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9181: Serialize TQueryCtx once per query
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14777 Change subject: IMPALA-9181: Serialize TQueryCtx once per query .. IMPALA-9181: Serialize TQueryCtx once per query When issuing Exec() rpcs to backends, we currently serialize the TQueryCtx once per backend. This is inefficient as the TQueryCtx is the same for all backends and really only needs to be serialized once. This patch serializes the TQueryCtx in the coordinator and then passes it to each BackendState when calling Exec(). Testing: - Passed full run of existing tests. - Single node perf run showed no significant change. Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6 --- M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-exec-mgr.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/test-env.cc M be/src/service/control-service.cc M be/src/service/control-service.h M common/protobuf/control_service.proto M common/thrift/ImpalaInternalService.thrift 13 files changed, 133 insertions(+), 108 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/14777/1 -- To view, visit http://gerrit.cloudera.org:8080/14777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6a4dd302fd5602ec2775492a041ddd51e7d7a6c6 Gerrit-Change-Number: 14777 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Jiawei Wang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14776 )
Change subject: IMPALA-7550: Add documentation to profile counters
..
Patch Set 1:
(6 comments)
Hi Tim, Lars, other Impala team members
This is the first edition of impala profile counters that I had based on Lars'
previous work on https://gerrit.cloudera.org/#/c/12116/
Thanks so much for Lars building the basis.
I had several comments on the commit itself. Any suggestions on how to improve
it is highly welcomed!
Thanks so much!
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.h@525
PS1, Line 525: /// Disk accessed bitmap
I am not too sure about the following two counters. Wondering if we should just
leave it like it is.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc@127
PS1, Line 127:
PROFILE_DEFINE_SUMMARY_STATS_COUNTER(ParquetUncompressedBytesReadPerColumn,
STABLE,
This seems redundant, any advice to modify it will be appreciated.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc@139
PS1, Line 139: PROFILE_DEFINE_COUNTER(DataCacheHitCount, STABLE, HIGH,
TUnit::UNIT,
Did not find the explanation for the following data cache counters. Might need
some other better explanation.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/scan-node.cc@82
PS1, Line 82: PROFILE_DEFINE_TIMER(RowBatchQueueGetWaitTime, UNSTABLE, LOW,
"Wall clock time that the "
Hi, Tim
I believe these are the Timer you mentioned in the threads. Any suggestions on
how should we handle it other than put it in the descriptions?
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@164
PS1, Line 164: enum class Significance {
I add this Significance to measure if the counter is important to a user. Maybe
we can also combine this with stability or redefine the stability? I am not too
sure about this.
http://gerrit.cloudera.org:8080/#/c/14776/1/www/profile_docs.tmpl
File www/profile_docs.tmpl:
http://gerrit.cloudera.org:8080/#/c/14776/1/www/profile_docs.tmpl@1
PS1, Line 1:
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14776 )
Change subject: IMPALA-7550: Add documentation to profile counters
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc@92
PS1, Line 92:
PROFILE_DEFINE_SUMMARY_STATS_COUNTER(InitialRangeIdealReservation, DEBUG, LOW,
TUnit::BYTES,
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@171
PS1, Line 171: // Medium level counters - somehow interesting counters to
monitor. It will probably be
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@296
PS1, Line 296: const
std::string& parent_counter_name = "") {
line too long (98 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7
Gerrit-Change-Number: 14776
Gerrit-PatchSet: 1
Gerrit-Owner: Jiawei Wang
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 21 Nov 2019 23:25:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Jiawei Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14776 Change subject: IMPALA-7550: Add documentation to profile counters .. IMPALA-7550: Add documentation to profile counters This is the work based on Lars' experiment on https://gerrit.cloudera.org/#/c/12116/ This commit did the following refactors on profile counters. 1. Add a singleton registry for runtime profile counters prototypes, similiar to what Kudu does for metrics. This allows us to generate profile documentation for all counters from the code. We add /profile_docs and a correspoding UI for the documentation of profile counters. 2. Profile counters are annotated with their stability: * Stable counters - generally useful to understand query performance, should only change rarely and if it does we'll make some effort to notify users. E.g. BytesRead. * Unstable but useful - useful to understand query performance, but subject to change, particularly if the implementation changes. E.g. RowBatchQueuePutWaitTime, MaterializeTupleTimer * Debugging counters - generally not useful to users of Impala, the main use case is low-level debugging. Can be hidden to reduce noise for most consumers of profiles. 3. Profile counters are also annotated with their significance to users. * Critical level counters - always useful on measuring query performance and status. Counters that everyone are interested. * High level counters - generally interesting counters. Most of the users will be interested and all the developers are very interested. * Medium level counters - somehow interesting counters to monitor. It will probably be interesting under some circumstance. Lot of developers are interested. * Low level counters - not interesting to users. Should be useful for developers to debug only. 4. We have around 250 counters. This commit did the replacement in scan-node and hdfs-scan-node-base and coordinator. This is still a WIP work and all the advices are welcomed! The downside is that we'd reduce the comments that currently explain some of the counters in header files by moving them to the .cc files. Additionally a (arguably good) limitation is that profile counter names need to be unique. Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 --- M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/scan-node.cc M be/src/exec/scan-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h D be/src/util/debug-counters.h M be/src/util/default-path-handlers.cc M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc A www/profile_docs.tmpl 12 files changed, 671 insertions(+), 296 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/14776/1 -- To view, visit http://gerrit.cloudera.org:8080/14776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 Gerrit-Change-Number: 14776 Gerrit-PatchSet: 1 Gerrit-Owner: Jiawei Wang
[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14632 ) Change subject: IMPALA-9126: part 1: hash join build partition cleanup .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb Gerrit-Change-Number: 14632 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 22:54:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14632 ) Change subject: IMPALA-9126: part 1: hash join build partition cleanup .. IMPALA-9126: part 1: hash join build partition cleanup Clarify some invariants (e.g. which join modes actually require build partition data to be attached to row batches). Move some logic for maintenance of the hash partitions to the builder. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb Reviewed-on: http://gerrit.cloudera.org:8080/14632 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/blocking-join-node.cc M be/src/exec/blocking-join-node.h A be/src/exec/join-op.h M be/src/exec/nested-loop-join-node.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 8 files changed, 166 insertions(+), 106 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb Gerrit-Change-Number: 14632 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14757 ) Change subject: IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh .. IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh Some jobs have been hanging in the testdata/bin/create-hbase.sh script. Logs during the hang show the HBase Master is stuck unitialized: Master startup cannot progress, in holding-pattern until region onlined. ... ERROR master.HMaster: Master failed to complete initialization after 90ms. Anecdotally, the HBase Master doesn't have this problem if we remove the /hbase/MasterProcWALs directory in kill-hbase.sh. This patch does exactly that. It is a hack, and we should update this code once we know what is going on. Testing: - test-with-docker.py fails without this patch and passes with it - Hand testing on my minicluster shows that this allows HBase to restart and be consistently usable Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Reviewed-on: http://gerrit.cloudera.org:8080/14757 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M testdata/bin/kill-hbase.sh 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: Joe McDonnell: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Gerrit-Change-Number: 14757 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14757 ) Change subject: IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Gerrit-Change-Number: 14757 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 22:43:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4618: Fixing #Hosts and adding #Instances in exec summary
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14715 ) Change subject: IMPALA-4618: Fixing #Hosts and adding #Instances in exec summary .. Patch Set 4: Code-Review+2 LGTM, please give the dev@ discussion another day or so before submitting. -- To view, visit http://gerrit.cloudera.org:8080/14715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bdf9a06d9bd842b2397cd16c28294b6bec7af69 Gerrit-Change-Number: 14715 Gerrit-PatchSet: 4 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 21 Nov 2019 22:37:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/14755 )
Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring
..
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state-map.h
File be/src/service/client-request-state-map.h:
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state-map.h@35
PS2, Line 35: Status AddClientRequestState(
After giving this more thought I'm skeptical of the fact that this map stores
shared_ptrs to non-const objects. Should we mention in the class comment that
users of the map need to synchronize access to the contained elements?
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state.h@53
PS2, Line 53: /// Execution state of the client-facing side a query. This
captures everything
side of a query
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state.h@188
PS2, Line 188: It is created
what is "It" here? The instance of this class, or query_ctx? Or should this say
"This method is called by.."?
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state.h@230
PS2, Line 230: const TExecRequest* exec_request() const { return
exec_request_.get(); }
Could add a DCHECK != nullptr and (nit) some whitespace around the method that
requires a comment (see above). The signature could also go back to const& as
it was before.
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/client-request-state.h@469
PS2, Line 469: std::unique_ptr exec_request_;
It is owned by this class, is the reason it cannot go back to a plain member
that its creation is deferred? We never release ownership of it at least, right?
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/impala-http-handler.h
File be/src/service/impala-http-handler.h:
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/impala-http-handler.h@57
PS2, Line 57: /// duplicate ClientRequestStates.
Mention thread safety concerns here, too?
Do we actually need shared ownership, i.e. can it happen that we store a CRS
here that the ImpalaServer has already released? If not, could we also store
plain pointers here and unique_ptr in the ImpalaServer to simplify ownership?
It looks like we only ever call DoFuncForAllEntries() on the map, so it might
not even have to be sharded?
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/14755/2/be/src/service/impala-server.cc@938
PS2, Line 938:
RETURN_IF_ERROR((*request_state)->CreateExecRequest(query_ctx));
I wonder if a comment here would help understand the effects of this call,
something like "calls into the frontend to get an ExecRequest" or similar. I
don't feel strongly about it though.
--
To view, visit http://gerrit.cloudera.org:8080/14755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca
Gerrit-Change-Number: 14755
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Thu, 21 Nov 2019 22:33:57 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table
..
Patch Set 4:
(4 comments)
Overall looks good to me. Thanks Vihang for fixing this!
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@383
PS4, Line 383: if (Boolean.parseBoolean(
:
getTblProperties().get(KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE))) {
: throw new AnalysisException(String.format("Table property
'%s' cannot be set to " +
: "true with an external Kudu table.",
KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE));
: }
This can be removed as 'external.table.purge' = true has been checked at L290?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS4, Line 2211: except we skip Kudu table creation if
:* it already exists
From the code in KuduCatalogOpExecutor, it looks like we skip Kudu table
creation (throw error) for both managed and external purge table?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java@92
PS4, Line 92: if table is externalPurgeTable
nit: if table is managed or externalPurgeTable?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/main/java/org/apache/impala/service/MetadataOp.java@21
PS4, Line 21: import java.util.Collections;
nit: why is the added while no other change in this file?
--
To view, visit http://gerrit.cloudera.org:8080/14750
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d
Gerrit-Change-Number: 14750
Gerrit-PatchSet: 4
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 21 Nov 2019 22:32:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. The goal is to let JDBC clients get constraint information from Impala tables. We implement two new metadata operations in impala-hs2-server, GetPrimaryKeys and GetCrossReference, which are already implemented in Hive's HS2. The thrift definitions are copied from Hive's TCLIService.thrift. In FE, these two operations are implemented to get the information from tables in the catalog. Much like GetColumns(), tables need to be loaded in order to be able to get PK/FK information. We wait for the PK table/FK table to load. In the implementation, PK/FK information is returned ONLY if the user has access to ALL the columns involved in the PK/FK relationship. Testing: - Added three test tables to our test datasets since most of our FE tests relied on dummy tables or testdata. It was difficult to test PK/FK with these methods. Also, we can build on this testdata in future when we make optimizer improvements. - Added unit tests in AuthorizationTest and JDBCtest. - Added e2e test in test_hs2.py - This patch modifies AnalyzeDDLTests and ToSqlTests to rely on the newly added dataset instead of dummy tables for pk/fk tests. Caveats: - Ranger needs OWNER user information for authorization. Since this is HMS metadata that we do not aggresively load, this information is not available for IncompleteTables. Some foreign key tables (fact tables for example) might have FK/PK relationships with several PK tables some of which might not be loaded in catalog. Currently we have no way to check column previleges without owner user information tables. We do not return keys involving such columns. Therefore, when Ranger is used, there maybe missing PK/FK relationships for parent tables that are not loaded. This can be tracked in IMPALA-9172. - Retrieval of constraints is not yet supported in LocalCatalog mode. See IMPALA-9158. Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Reviewed-on: http://gerrit.cloudera.org:8080/14720 Reviewed-by: Vihang Karajgaonkar Tested-by: Impala Public Jenkins --- M be/src/rpc/kerberos-test.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M bin/rat_exclude_files.txt M common/thrift/Frontend.thrift M common/thrift/hive-1-api/TCLIService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java M testdata/data/README A testdata/data/child_table.txt A testdata/data/parent_table.txt A testdata/data/parent_table_2.txt M testdata/datasets/functional/functional_schema_template.sql M tests/hs2/test_hs2.py 25 files changed, 835 insertions(+), 48 deletions(-) Approvals: Vihang Karajgaonkar: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 13 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14629 ) Change subject: IMPALA-9100: Handle duplicate occurrences of flags for tests/run-tests.py .. Patch Set 2: Code-Review+2 Carry +2 -- To view, visit http://gerrit.cloudera.org:8080/14629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I60dc9a898f69804e2a53c05b5dfab2f948a22097 Gerrit-Change-Number: 14629 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 21 Nov 2019 22:25:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. Patch Set 12: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 22:25:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#9). Change subject: IMPALA-9126: part 2: no hash join probe structures in build .. IMPALA-9126: part 2: no hash join probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 185 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/9 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hash join probe structures in build .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG@7 PS8, Line 7: part 2: no hj probe structures in build > nit: I would prefer to spell out hash join to make it clearer to people who Done -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 22:17:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14750 ) Change subject: IMPALA-9092: Add support for creating external Kudu table .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5105/ : 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/14750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d Gerrit-Change-Number: 14750 Gerrit-PatchSet: 4 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 22:07:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table
..
Patch Set 4: Code-Review+1
(2 comments)
LGTM, just a couple tiny things that I don't feel too strongly about. Would be
great if Hao and/or Grant did a pass too.
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@499
PS4, Line 499: isSynchronizedTable
nit: maybe update this too, and TestCreateSynchronizedKuduTable, to refer to
external purge tables?
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java:
http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@291
PS4, Line 291: Assert.assertTrue("Synchronized Kudu tables in Hive-3 must
contain external.table"
: + ".purge table property",
output.contains("'external.table.purge'='TRUE'"));
Do you think it's worth checking that the table property TRANSLATED_TO_EXTERNAL
is missing?
--
To view, visit http://gerrit.cloudera.org:8080/14750
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d
Gerrit-Change-Number: 14750
Gerrit-PatchSet: 4
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 21 Nov 2019 22:03:01 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5273/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 21 Nov 2019 21:16:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 7: Code-Review+2 Thanks for making the changes. The patch looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 21 Nov 2019 21:15:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 8 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 21 Nov 2019 21:16:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14750 ) Change subject: IMPALA-9092: Add support for creating external Kudu table .. Patch Set 4: (33 comments) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@71 PS4, Line 71: super.AnalysisError(appendSynchronizedTblProps(stmt, isExternalPurgeTbl), expectedError); line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@102 PS4, Line 102: "range(x, y) (partition value = (1+1, 2+2), partition value = ((1+1+1)+1, 10), " + line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@221 PS4, Line 221: "(PARTITION VALUE = 'abc')' is not a key column. Only key columns can be used " line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@277 PS4, Line 277: "partition by range(a, b) (partition (0, 0) < values <= (1, 1)) stored as kudu", line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@286 PS4, Line 286: "partitioning columns: (1 vs 2). Range partition: 'PARTITION 0 < VALUES <= 1'", line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java File fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java: http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@270 PS4, Line 270: " id INT NOT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@271 PS4, Line 271: " bool_col BOOLEAN NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (99 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@272 PS4, Line 272: " tinyint_col TINYINT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (102 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@273 PS4, Line 273: " smallint_col SMALLINT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (104 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@274 PS4, Line 274: " int_col INT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@275 PS4, Line 275: " bigint_col BIGINT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@276 PS4, Line 276: " float_col FLOAT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@277 PS4, Line 277: " double_col DOUBLE NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@278 PS4, Line 278: " date_string_col STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (105 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@279 PS4, Line 279: " string_col STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@280 PS4, Line 280: " timestamp_col TIMESTAMP NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,\n" + line too long (106 > 90) http://gerrit.cloudera.org:8080/#/c/14750/4/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@281 PS4, Line 281: " year INT NULL ENCODING AUTO_ENCODING
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5104/ : 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/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 21 Nov 2019 21:14:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9092: Add support for creating external Kudu table
Vihang Karajgaonkar has uploaded a new patch set (#4). (
http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table
..
IMPALA-9092: Add support for creating external Kudu table
In HMS-3 the translation layer converts a managed kudu table into a external
kudu table
and adds additional table property 'external.table.purge' to 'true'. This means
any
installation which is using HMS-3 (or a Hive version which has HIVE-22158) will
always
create Kudu tables as external tables. This is problematic since the the output
of show
create table will now be different and may confuse the users.
In order to improve the user experience of such synchronized tables (external
tables with
external.table.purge property set to true), this patch adds support in Impala
to create
external Kudu tables. Previous versions of Impala disallowed creating a
external Kudu
table if the Kudu table did not exist. After this patch, Impala will check if
the Kudu
table exists and if it does not it will create a Kudu table based on the schema
provided
in the create table statement. The command will error out if the Kudu table
already
exists. However, this applies to only the synchronized tables. Previous way to
create a
pure external table behaves the same.
Following syntax of creating a synchronized table is now allowed:
CREATE EXTERNAL TABLE foo (
id int PRIMARY KEY,
name string)
PARTITION BY HASH PARTITIONS 8
STORED AS KUDU
TBLPROPERTIES ('external.table.purge'='true')
The syntax is very similar to creating a managed table, except for the EXTERNAL
keyword
and additional table property. A synchronized table will behave similar to
managed Kudu
tables (drops and renames are allowed). The output of show create table on a
synchronized
table will display the full column and partition spec similar to the managed
tables.
Testing:
1. After the CDP version bump all of the existing Kudu tables now create
synchronized
tables so there is good coverage there.
2. Added additional tests which create synchronized tables and compares the
show create
table output.
3. Ran exhaustive tests with both CDP and CDH builds.
Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
M tests/common/skip.py
M tests/metadata/test_show_create_table.py
M tests/query_test/test_kudu.py
16 files changed, 794 insertions(+), 262 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/14750/4
--
To view, visit http://gerrit.cloudera.org:8080/14750
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d
Gerrit-Change-Number: 14750
Gerrit-PatchSet: 4
Gerrit-Owner: Vihang Karajgaonkar
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8867: Further deflake test auto scaling
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14740 ) Change subject: IMPALA-8867: Further deflake test_auto_scaling .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29808982cc6226152c544cb99f76961b582975a7 Gerrit-Change-Number: 14740 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 21 Nov 2019 21:03:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8867: Further deflake test auto scaling
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14740 ) Change subject: IMPALA-8867: Further deflake test_auto_scaling .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5272/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29808982cc6226152c544cb99f76961b582975a7 Gerrit-Change-Number: 14740 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 21 Nov 2019 21:03:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9127: explicit probe state machine in hash join
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14688 ) Change subject: IMPALA-9127: explicit probe state machine in hash join .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064 Gerrit-Change-Number: 14688 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:56:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9085: [DOCS] Refactored impala s3.xml
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14627 ) Change subject: IMPALA-9085: [DOCS] Refactored impala_s3.xml .. Patch Set 4: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/535/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib274968a0412b4b8757f31ab674d4b82311de70a Gerrit-Change-Number: 14627 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 21 Nov 2019 20:56:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 8: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG@7 PS8, Line 7: part 2: no hj probe structures in build nit: I would prefer to spell out hash join to make it clearer to people who are just browsing git log --oneline, e.g. "Remove probe structures from hash join builder". -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:39:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9085: [DOCS] Refactored impala s3.xml
Hello Sahil Takiar, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14627 to look at the new patch set (#4). Change subject: IMPALA-9085: [DOCS] Refactored impala_s3.xml .. IMPALA-9085: [DOCS] Refactored impala_s3.xml Change-Id: Ib274968a0412b4b8757f31ab674d4b82311de70a --- M docs/shared/impala_common.xml M docs/topics/impala_s3.xml 2 files changed, 352 insertions(+), 542 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/14627/4 -- To view, visit http://gerrit.cloudera.org:8080/14627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib274968a0412b4b8757f31ab674d4b82311de70a Gerrit-Change-Number: 14627 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9085: [DOCS] Refactored impala s3.xml
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14627 ) Change subject: IMPALA-9085: [DOCS] Refactored impala_s3.xml .. Patch Set 4: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/535/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib274968a0412b4b8757f31ab674d4b82311de70a Gerrit-Change-Number: 14627 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 21 Nov 2019 20:29:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Hello Quanlong Huang, Yongzhi Chen, Vihang Karajgaonkar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14611 to look at the new patch set (#7). Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. IMPALA-9110: Add table loading time break-down metrics for HdfsTable A. Problem: Catalog table loading currently only records the total loading time. We will need some break-down times, i.e. more detailed time recording on each loading function. Also, the table schema loading is not taken into account for load-duration. We will need to add some more metrics for that. B. Solution: - We added "hms-load-tbl-schema", "load-duration.all-column-stats", "load-duration.all-partitions.total-time", "load-duration.all-partitions.file-metadata". Also, we logged the loadValidWriteIdList() time. So now we have a more detailed breakdown time for table loading info. The table loading time metrics for HDFS tables are in the following hierarchy: - Table Schema Loading - Table Metadata Loading - total time - all column stats loading time - ValidWriteIds loading time - all partitions loading time - total time - file metadata loading time - storage-metadata-loading-time(standalone metric) 1. Table Schema Loading: * Meaning: The time for HMS to fetch table object and the real schema loading time. Normally, the code path is "msClient.getHiveClient().getTable(dbName, tblName)" * Metric : hms-load-tbl-schema 2. Table Metadata Loading -- total time * Meaning: The time to load all the table metadata. The code path is load() function in HdfsTable.load() function. * Metric: load-duration.total-time 2.1 Table Metadata Loading -- all column stats * Meaning: load all column stats, this is part of table metadata loading The code path is HdfsTable.loadAllColumnStats() * Metric: load-duration.all-column-stats 2.2 Table Metadata Loading -- loadValidWriteIdList * Meaning: fetch ValidWriteIds from HMS The code path is HdfsTable.loadValidWriteIdList() * Metric: no metric recorded for this one. Instead, a debug log is generated. 2.3 Table Metadata Loading -- storage metadata loading(standalone metric) * Meaning: Storage related to file system operations during metadata loading.(The amount of time spent loading metadata from the underlying storage layer.) * Metric: we rename it to load-duration.storage-metadata. This is a metric introduced by IMPALA-7322 2.4 Table Metadata Loading -- load all partitions * Meaning: Load all partitions time, including fetching all partitions from HMS and loading all partitions. The code path is MetaStoreUtil.fetchAllPartitions() and HdfsTable.loadAllPartitions() * Metric: load-duration.all-partitions 2.4.1 Table Metadata Loading -- load all partitions -- load file metadata * Meaning: The file metadata loading for all all partitions. (This is part of 2.4). Code path: loadFileMetadataForPartitions() inside loadAllPartitions() * Metric: load-duration.all-partitions.file-metadata C. Extra thing in this commit: 1. Add PrintUtils.printTimeNs for PrettyPrint time in FrontEnd 2. Add explanation for table loading manager D. Test: 1. Add Unit tests for PrintUtils.printTime() function 2. Manual describe table and verify the table loading metrics are correct. Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/common/PrintUtils.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java 10 files changed, 186 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/14611/7 -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-9110: Add table loading time break-down metrics for HdfsTable
Jiawei Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14611 ) Change subject: IMPALA-9110: Add table loading time break-down metrics for HdfsTable .. Patch Set 7: (3 comments) Thanks for the review. Let me know if we need add anything else in this commit. http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@163 PS6, Line 163: load-duration.all-partitions"; > I understand that the naming is used to represent nesting of the metrics. S Done http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/Table.java@149 PS6, Line 149: load-duration"; > Since this is the top level load-duration adding a specific --total-time is Done http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java File fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java: http://gerrit.cloudera.org:8080/#/c/14611/6/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java@262 PS6, Line 262: There is a discussion here: https://issues.apache.org/jira/brows > May be just say (See discussion in ..) :) Done -- To view, visit http://gerrit.cloudera.org:8080/14611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5381f9316df588b2004876c6cd9fb7e674085b10 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 7 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 21 Nov 2019 20:28:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/534/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 21 Nov 2019 20:21:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5102/ : 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/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:04:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14755 ) Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5271/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 21 Nov 2019 20:05:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/14755 )
Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/client-request-state-map.h
File be/src/service/client-request-state-map.h:
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/client-request-state-map.h@35
PS1, Line 35: const std::shared_ptr& request_state) {
> We should take the shared_ptr by value since we are going to retain a copy
Done
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/client-request-state-map.h@44
PS1, Line 44: ss << "query id " << PrintId(query_id) << " already exists";
> This could use "Substitute()" to be a bit more concise
Done
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/client-request-state-map.h@55
PS1, Line 55: Status DeleteClientRequestState(
> You could add an overload without the request_state argument to make the us
Done
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/impala-http-handler.h
File be/src/service/impala-http-handler.h:
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/impala-http-handler.h@55
PS1, Line 55: /// Tracks the ClientRequestStates of the currently registered
queries.
> Can you add to the comment why we store them here as well as in the ImpalaS
Done
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/14755/1/be/src/service/impala-server.h@1084
PS1, Line 1084: ClientRequestState is owned
> What does this mean in the context of shared_ptr? If it is owned, shouldn't
Changed to unique_ptr and did a bit more re-factoring so that the
ClientRequestState creates the TExecRequest internally.
--
To view, visit http://gerrit.cloudera.org:8080/14755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca
Gerrit-Change-Number: 14755
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Thu, 21 Nov 2019 20:02:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5101/ : 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/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:02:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Hello Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14755 to look at the new patch set (#2). Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. IMPALA-9124: ImpalaServer and ClientRequestState refactoring Re-factoring several areas of the ImpalaServer and ClientRequestState in preparation for future work required for transparent query retries. Re-factored TExecRequest ownership in ClientRequestState. Previously, the TExecRequest was passed to the ClientRequestState via the Exec(TExecRequest) method. Now it is passed via the constructor and its lifetime is managed by an unique_ptr. Using an unique_ptr avoids a copy of the TExecRequest that was happening in ClientRequestState::Exec(TExecRequest) and clarifies the ownership / lifecycle of a TExecRequest. Re-factored ImpalaServer::UnregisterQuery into multiple methods. Made ClientRequestStateMap a dedicated class that extends ShardedQueryMap with additional methods to add and delete a ClientRequestState. The re-factoring breaks up the large UnregisterQuery method into multiple smaller method and adds some additional code documentation. Re-factored ImpalaHttpHandler's usage of the ClientRequestStateMap so that it tracks its own list of running query ids / ClientRequestStates rather than using the one owned by ImpalaServer. This is particularly important for transparent query retries because retrying a query causes duplicate ClientRequestStates to be added to the ImpalaServer::client_request_state_map_. Testing: * Ran core tests Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca --- M be/src/service/child-query.cc A be/src/service/client-request-state-map.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h 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 8 files changed, 230 insertions(+), 118 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/14755/2 -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/7/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14716/7/be/src/exec/partitioned-hash-join-builder.h@128 PS7, Line 128: int GetNumSpilledHashPartitions() const; This no longer needs to be public. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 19:42:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Hello Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#8). Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 185 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/8 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/534/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 21 Nov 2019 19:34:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Hello Gabor Kaszab, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14722 to look at the new patch set (#2). Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 - The following format specifiers are documented: - FX - FM - Free text Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 --- M docs/topics/impala_conversion_functions.xml 1 file changed, 380 insertions(+), 380 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/2 -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Alex Rodoni has posted comments on this change. (
http://gerrit.cloudera.org:8080/14722 )
Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
..
Patch Set 1:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@552
PS1, Line 552:
: Must be specified at the beginning
of the format
: pattern and is
valid for the
: whole pattern.
:
> This statement is true for both direction of the conversion not just for st
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@558
PS1, Line 558: In string to date/time conversions, this forces
: strict separator matching and
expects all the tokens
: in the input to have the same length
as their
: maximum length. For example, a month
has to be of
: length 2 prefixed by zero if
needed.
> I'd split this into two items:
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@564
PS1, Line 564:
: Numeric output is left padded with
zeros.
:
> This is a valid statement but for datetime to string path.
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@567
PS1, Line 567:
: Non-numeric output except for
:
AM/PM is right
: padded with spaces up to the
expected length.
:
> In milestone 1 and 2 there are no tokens with non-numeric output (except AM
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@572
PS1, Line 572:
: Separators must match exactly,
including the
: separator character.
:
> I'd merge this item with the one at the top of the list that mentions stric
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@588
PS1, Line 588: Suppresses blank padding for the element immediately
: following an FM in the
: pattern string:
: Trailing spaces for text output
: Leading zeroes for numeric
output
:
> This is valid only for the datetime to string path.
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@629
PS1, Line 629: datet/ime
> nit: typo
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@635
PS1, Line 635: except the case may differ.
> Case also has to match.
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@643
PS1, Line 643: '-MM-DD"\"text\""'
> The format part is incorrect in this example. It should be surrounded by do
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@646
PS1, Line 646: To include a literal double quote character
: within the nested string, the
double quote
: character must be escaped with a
double backslash
: (\\").
> I think there is a confusion in the design doc about how to include literal
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@652
PS1, Line 652: If the whole pattern string
: is delimited by double quotes,
escape with a
: triple backslash: (\\\")
> Let me provide the same example here that I gave above. The difference is t
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@665
PS1, Line 665: an error
: returns.
> an error returns as it's not trivial to find where the separator sequence e
Done
http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@792
PS1, Line 792: CAST('2019-11-10text'
:AS DATE
:FORMAT '-MM-DD"text"')
> Instead of this example I'd use something that seems more real-life e.g.:
Done
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Hello Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#6). Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 186 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/6 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14716 )
Change subject: IMPALA-9126: part 2: no hj probe structures in build
..
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@118
PS5, Line 118: /// new partitions with level input_partition->leve() + 1. The
previous hash partitions
> typo: missing "l"
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@329
PS5, Line 329: m
> nit: capital M
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@483
PS5, Line 483: //
> nit: missing /
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@987
PS5, Line 987: probe_hash_partitions_.resize(PARTITION_FANOUT);
: bool have_spilled_hash_partitions = false;
: for (int i = 0; i < PARTITION_FANOUT; ++i) {
: PhjBuilder::Partition* build_partition =
builder_->hash_partition(i);
: if (build_partition->IsClosed() ||
!build_partition->is_spilled()) continue;
: have_spilled_hash_partitions = true;
: DCHECK(!probe_streams.empty()) << "Builder should have
created enough streams";
: CreateProbePartition(i, std::move(probe_streams.back()));
: probe_streams.pop_back();
: }
: DCHECK(probe_streams.empty()) << "Builder should not have
created extra streams";
> This loop and AllocateProbeStreams() feel redundant - CreateProbePartition
That's a good point. I ended up restructuring this so that it's more consistent
with other code - it creates the Partitions in-place in probe_hash_partitions_
and initialises them with a PrepareForWrite() method. Much more concise.
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@1055
PS5, Line 1055: for (auto& stream : *streams) {
: stream->Close(nullptr,
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
: }
> I would prefer to avoid the gotos by doing cleanup on the caller side or ad
This got fixed as a consequence of the other refactoring.
--
To view, visit http://gerrit.cloudera.org:8080/14716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d
Gerrit-Change-Number: 14716
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 21 Nov 2019 19:33:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9146: Add a configurable limit for the size of broadcast input.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14690 ) Change subject: IMPALA-9146: Add a configurable limit for the size of broadcast input. .. Patch Set 8: Hit IMPALA-9152 and IMPALA-9165. I might wait until Joe's fix for IMPALA-9165 is merged before retrying. -- To view, visit http://gerrit.cloudera.org:8080/14690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5639ca38acb72e0194aa80bc6ebb6cafb2acd9 Gerrit-Change-Number: 14690 Gerrit-PatchSet: 8 Gerrit-Owner: Aman Sinha Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 19:15:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9177: Bump min-query-mem-limit for dockerised tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14759 ) Change subject: IMPALA-9177: Bump min-query-mem-limit for dockerised tests .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5270/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f478b988a7bc2e7bd7422ceda52c6eddac04d0a Gerrit-Change-Number: 14759 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:57:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14757 ) Change subject: IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5100/ : 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/14757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Gerrit-Change-Number: 14757 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:52:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5099/ : 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/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 18:22:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/14716 )
Change subject: IMPALA-9126: part 2: no hj probe structures in build
..
Patch Set 5: Code-Review+1
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@118
PS5, Line 118: /// new partitions with level input_partition->leve() + 1. The
previous hash partitions
typo: missing "l"
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@329
PS5, Line 329: m
nit: capital M
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@483
PS5, Line 483: //
nit: missing /
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@987
PS5, Line 987: probe_hash_partitions_.resize(PARTITION_FANOUT);
: bool have_spilled_hash_partitions = false;
: for (int i = 0; i < PARTITION_FANOUT; ++i) {
: PhjBuilder::Partition* build_partition =
builder_->hash_partition(i);
: if (build_partition->IsClosed() ||
!build_partition->is_spilled()) continue;
: have_spilled_hash_partitions = true;
: DCHECK(!probe_streams.empty()) << "Builder should have
created enough streams";
: CreateProbePartition(i, std::move(probe_streams.back()));
: probe_streams.pop_back();
: }
: DCHECK(probe_streams.empty()) << "Builder should not have
created extra streams";
This loop and AllocateProbeStreams() feel redundant - CreateProbePartition
doesn't seem to do anything fancy, it just constructs a ProbePartition without
side effects.
I am not sure about the best way to merge them - maybe CreateProbePartition
could handle both the allocation and the ProbePartition construction + the
cleanup on error stuff could be done using the members of
probe_hash_partitions_.
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@1055
PS5, Line 1055: for (auto& stream : *streams) {
: stream->Close(nullptr,
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
: }
I would prefer to avoid the gotos by doing cleanup on the caller side or adding
a wrapper function.
--
To view, visit http://gerrit.cloudera.org:8080/14716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d
Gerrit-Change-Number: 14716
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Thu, 21 Nov 2019 18:10:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 1: hash join build partition cleanup
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14632 ) Change subject: IMPALA-9126: part 1: hash join build partition cleanup .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5269/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14632 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife8d0fa5dd14c7d3f3d726dd38c07d8cbceabadb Gerrit-Change-Number: 14632 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:09:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9177: Bump min-query-mem-limit for dockerised tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14759 ) Change subject: IMPALA-9177: Bump min-query-mem-limit for dockerised tests .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14759 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f478b988a7bc2e7bd7422ceda52c6eddac04d0a Gerrit-Change-Number: 14759 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:08:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14757 ) Change subject: IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5268/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Gerrit-Change-Number: 14757 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:08:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14757 ) Change subject: IMPALA-9165: Hack to remove MasterProcWALs directory in kill-hbase.sh .. Patch Set 2: Code-Review+2 Forgot to include the " || true" in the first upload. This fixes that. Carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/14757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icef3d30e6b539a175e03f63fcdbfb2d4608c08fa Gerrit-Change-Number: 14757 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 18:08:11 + Gerrit-HasComments: No
