[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 5: Code-Review+1 (5 comments) http://gerrit.cloudera.org:8080/#/c/7414/5/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 101 Unnecessary include because of std::hash Line 117 MSVC stuff is removed Line 242 MSVC stuff is removed Line 305 MSVC stuff is removed Line 318 MSVC stuff is removed -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin ChulGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7414 to look at the new patch set (#5). Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 37 insertions(+), 185 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/5 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin ChulGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5109: Increase range of backend latency histogram
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/7676 Change subject: IMPALA-5109: Increase range of backend latency histogram .. IMPALA-5109: Increase range of backend latency histogram Tiny change to increase range of backend start-up latency histogram to 30 minutes, with 4 s.f. of precision. This allows us to capture situations where some backend is very slow to start with more discrimination than the previous 20s upper bound. Change-Id: I7b6f39c0f769b5b590009d47f5b2e904f48fae8d --- M be/src/runtime/coordinator.cc 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/7676/1 -- To view, visit http://gerrit.cloudera.org:8080/7676 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7b6f39c0f769b5b590009d47f5b2e904f48fae8d Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1057/ -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Henry Robinson has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 6: Code-Review+2 Fix a couple of clang-tidy warnings. -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Hello Impala Public Jenkins, Sailesh Mukil, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7606 to look at the new patch set (#6). Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. IMPALA-5743: Support TLS version configuration for Thrift servers * Add --ssl_minimum_version which controls the minimum SSL/TLS version that clients and servers will use when negotiating a secure connection. * Two kinds of version specification are allowed: 'TLSv1.1' enables TLSv1.1 and all subsequent verisons. 'TLSv1.1_only' enables only TLSv1.1. The latter is not exposed in user-facing text as it is typically only used for testing. * Handle case where platform may not support TLSv1.1 or v1.2 by checking OpenSSL version number. * Bump Thrift toolchain version to -p10. Testing: * New tests in thrift-server-test.cc. In particular, test all 36 configurations of client and server protocol versions, and ensure that the expected successes or failures are seen. Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 --- M be/src/catalog/catalogd-main.cc M be/src/rpc/thrift-client.cc M be/src/rpc/thrift-client.h M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/service/impala-server.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestored-main.cc M bin/impala-config.sh 10 files changed, 215 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/7606/6 -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1056/ -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2
Hello Sailesh Mukil, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7675 to look at the new patch set (#2). Change subject: IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 .. IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 The shell uses Thrift's TSSLSocket to negotiate secure connections to Impala. This socket uses a variable SSL_VERSION to determine which SSL and TLS protocol versions it will connect to. SSL_VERSION was hardcoded to be PROTOCOL_TLSv1, which only supports TLSv1 servers and no other protocol version. Change the allowed version to be PROTOCOL_SSLv23, which supports any TLS or SSL protocol. We rely on the server not to allow SSLv2 or v3 connections. Testing: Added a new custom cluster test to confirm that the shell can connect to a TLSv1.2 cluster. Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 --- M shell/TSSLSocketWithWildcardSAN.py M tests/custom_cluster/test_client_ssl.py M tests/util/thrift_util.py 3 files changed, 26 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/7675/2 -- To view, visit http://gerrit.cloudera.org:8080/7675 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2
Henry Robinson has posted comments on this change. Change subject: IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7675/1/tests/custom_cluster/test_client_ssl.py File tests/custom_cluster/test_client_ssl.py: Line 103: > Add: Done -- To view, visit http://gerrit.cloudera.org:8080/7675 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. Fix: If the source URI does not have write permissions we get a "READ WRITE warning" on CREATE TABLE LIKE PARQUET. During analyze() in the class CreateTableLikeFileStmt we should be checking only for "READ" permission for the source URI not for READ and WRITE as the source URI should be readable. Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Reviewed-on: http://gerrit.cloudera.org:8080/7671 Reviewed-by: Sailesh MukilReviewed-by: Bharath Vissapragada Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Impala Public Jenkins: Verified Bharath Vissapragada: Looks good to me, approved Sailesh Mukil: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath VissapragadaGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1478: Improve error message when subquery is used in the ON clause
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-1478: Improve error message when subquery is used in the ON clause .. IMPALA-1478: Improve error message when subquery is used in the ON clause Fix: Print the error stating that "Suquery not allowed in the ON clause" Add test case for testing the failure when a subquery is used in the ON clause. Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2 Reviewed-on: http://gerrit.cloudera.org:8080/7588 Reviewed-by: Bharath VissapragadaTested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java 2 files changed, 9 insertions(+), 0 deletions(-) Approvals: Impala Public Jenkins: Verified Bharath Vissapragada: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pranay Singh
[Impala-ASF-CR] IMPALA-1478: Improve error message when subquery is used in the ON clause
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-1478: Improve error message when subquery is used in the ON clause .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Alex BehmGerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pranay Singh Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/7675/1/tests/custom_cluster/test_client_ssl.py File tests/custom_cluster/test_client_ssl.py: Line 103: Add: @pytest.mark.xfail(run=True, reason="IMPALA-4295 on Centos6") Certain wildcard certs don't work on CentOS 6. It still runs and passes on other platforms. -- To view, visit http://gerrit.cloudera.org:8080/7675 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/7675 Change subject: IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 .. IMPALA-5775: Allow shell to support TLSv1, v1.1 and v1.2 The shell uses Thrift's TSSLSocket to negotiate secure connections to Impala. This socket uses a variable SSL_VERSION to determine which SSL and TLS protocol versions it will connect to. SSL_VERSION was hardcoded to be PROTOCOL_TLSv1, which only supports TLSv1 servers and no other protocol version. Change the allowed version to be PROTOCOL_SSLv23, which supports any TLS or SSL protocol. We rely on the server not to allow SSLv2 or v3 connections. Testing: Added a new custom cluster test to confirm that the shell can connect to a TLSv1.2 cluster. Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 --- M shell/TSSLSocketWithWildcardSAN.py M tests/custom_cluster/test_client_ssl.py M tests/util/thrift_util.py 3 files changed, 25 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/7675/1 -- To view, visit http://gerrit.cloudera.org:8080/7675 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5487f82d110676b9c3c7a5305931da00c7f68ca0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() Plumbing statuses through TextConverter::CodegenWriteSlot(), which eventually propagate to the runtime profile. Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Reviewed-on: http://gerrit.cloudera.org:8080/7574 Reviewed-by: Tim ArmstrongTested-by: Impala Public Jenkins --- M be/src/exec/hdfs-scanner.cc M be/src/exec/text-converter.cc M be/src/exec/text-converter.h 3 files changed, 53 insertions(+), 41 deletions(-) Approvals: Impala Public Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5776: Write partial tuple to the correct mempool
Taras Bobrovytsky has uploaded a new patch set (#4). Change subject: IMPALA-5776: Write partial tuple to the correct mempool .. IMPALA-5776: Write partial tuple to the correct mempool In the text scanner, we were writing the partial tuple variable length data to data_buffer_pool_ mempool which caused strange behavior, such as incorrect results. If we are scanning compressed data, the pool gets attached to the row batch at the end of a GetNext() call and gets freed before the next GetNext() call. This is wrong because we expect the data in the partial tuple to survive between the GetNext() calls. If we are scanning non compressed data, data_buffer_pool_ never gets cleared and grows over time until the scanner finishes reading the scan range. We fix the problem by creating a new memory pool for the partial tuple constant and variable length data. The new memory pool does not hold tuple data of returned batches because the data is always deep-copied into the output batch. Testing: - Updated test_scanners_fuzz.py to make slightly more significant changes to the data files. - Ran scanner tests locally on ASAN build. - No new tests were added, because it is difficult to construct test cases due to the issue being non-deterministic. Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 --- M be/src/exec/hdfs-text-scanner.cc M be/src/exec/hdfs-text-scanner.h M tests/query_test/test_scanners_fuzz.py 3 files changed, 46 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/7639/4 -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5776: Write partial tuple to the correct mempool
Taras Bobrovytsky has uploaded a new patch set (#4). Change subject: IMPALA-5776: Write partial tuple to the correct mempool .. IMPALA-5776: Write partial tuple to the correct mempool In the text scanner, we were writing the partial tuple variable length data to data_buffer_pool_ mempool which caused strange behavior, such as incorrect results. If we are scanning compressed data, the pool gets attached to the row batch at the end of a GetNext() call and gets freed before the next GetNext() call. This is wrong because we expect the data in the partial tuple to survive between the GetNext() calls. If we are scanning non compressed data, data_buffer_pool_ never gets cleared and grows over time until the scanner finishes reading the scan range. We fix the problem by creating a new memory pool for the partial tuple constant and variable length data. The new memory pool does not hold tuple data of returned batches because the data is always deep-copied into the output batch. Testing: - Updated test_scanners_fuzz.py to make slightly more significant changes to the data files. - Ran scanner tests locally on ASAN build. - No new tests were added, because it is difficult to construct test cases due to the issue being non-deterministic. Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 --- M be/src/exec/hdfs-text-scanner.cc M be/src/exec/hdfs-text-scanner.h M tests/query_test/test_scanners_fuzz.py 3 files changed, 46 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/7639/4 -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5776: Write partial tuple to the correct mempool
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-5776: Write partial tuple to the correct mempool .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/7639/3/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: Line 804: // Copy over all materialized slots in the partial tuple. > Only describes what the code does. How about something like: Done Line 808: // prevent the accumulation of variable length data in it. We also recreate the > The partial tuple isn't re-created here Done http://gerrit.cloudera.org:8080/#/c/7639/3/be/src/exec/hdfs-text-scanner.h File be/src/exec/hdfs-text-scanner.h: Line 186: /// Utility function to write out 'num_fields' to 'tuple_'. This is used to parse > Comment is weird, I suggest rephrasing to something like: Done Line 227: /// Mem pool for the partial tuple. > Mention that it does not hold tuple data of returned batches because the da Done Line 230: /// Memory to store partial tuples split across buffers. Memory comes from > Mention that this tuple is always deep copied into the output batch Done Line 231: /// partial_tuple_pool_. There is only one tuple allocated for this object and reused > The 2nd sentence does not seem to apply anymore because we realloc the part Done -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5776: Write partial tuple to the correct mempool
Taras Bobrovytsky has uploaded a new patch set (#4). Change subject: IMPALA-5776: Write partial tuple to the correct mempool .. IMPALA-5776: Write partial tuple to the correct mempool In the text scanner, we were writing the partial tuple variable length data to data_buffer_pool_ mempool which caused strange behavior, such as incorrect results. If we are scanning compressed data, the pool gets attached to the row batch at the end of a GetNext() call and gets freed before the next GetNext() call. This is wrong because we expect the data in the partial tuple to survive between the GetNext() calls. If we are scanning non compressed data, data_buffer_pool_ never gets cleared and grows over time until the scanner finishes reading the scan range. We fix the problem by creating a new memory pool for the partial tuple will contains partial tuple contant length and variable length data. Testing: - Updated test_scanners_fuzz.py to make slightly more significant changes to the data files. - Ran scanner tests locally on ASAN build. - No new tests were added, because it is difficult to construct test cases due to the issue being non-deterministic. Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 --- M be/src/exec/hdfs-text-scanner.cc M be/src/exec/hdfs-text-scanner.h M tests/query_test/test_scanners_fuzz.py 3 files changed, 46 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/7639/4 -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1056/ -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Hello Sailesh Mukil, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7606 to look at the new patch set (#5). Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. IMPALA-5743: Support TLS version configuration for Thrift servers * Add --ssl_minimum_version which controls the minimum SSL/TLS version that clients and servers will use when negotiating a secure connection. * Two kinds of version specification are allowed: 'TLSv1.1' enables TLSv1.1 and all subsequent verisons. 'TLSv1.1_only' enables only TLSv1.1. The latter is not exposed in user-facing text as it is typically only used for testing. * Handle case where platform may not support TLSv1.1 or v1.2 by checking OpenSSL version number. * Bump Thrift toolchain version to -p10. Testing: * New tests in thrift-server-test.cc. In particular, test all 36 configurations of client and server protocol versions, and ensure that the expected successes or failures are seen. Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 --- M be/src/catalog/catalogd-main.cc M be/src/rpc/thrift-client.cc M be/src/rpc/thrift-client.h M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/service/impala-server.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestored-main.cc M bin/impala-config.sh 10 files changed, 212 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/7606/5 -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8
Jim Apple has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 .. Patch Set 17: > That build included this patch. Or do you have reason to believe it > didn't? Ah, OK. That's a part of Gerrit I have not yet understood; thanks for the clarification! -- To view, visit http://gerrit.cloudera.org:8080/5716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8
Henry Robinson has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 .. Patch Set 17: That build included this patch. Or do you have reason to believe it didn't? -- To view, visit http://gerrit.cloudera.org:8080/5716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8
Jim Apple has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 .. Patch Set 17: > Verified by build: https://jenkins.impala.io/job/gerrit-verify-dryrun/1051/ I'm confused. That build verified https://gerrit.cloudera.org/#/c/5717/19 -- To view, visit http://gerrit.cloudera.org:8080/5716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build
Henry Robinson has submitted this change and it was merged. Change subject: IMPALA-4669: [SECURITY] Add security library to build .. IMPALA-4669: [SECURITY] Add security library to build * Minor compilation fix * Add krb5 as a non-toolchain dependency * Handle legacy versions of libkrb5.so by providing implementation of krb5_is_config_principal(). * Link against openssl from the toolchain if 1.0.0 or higher not found on build machine. * Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}. Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Reviewed-on: http://gerrit.cloudera.org:8080/5717 Reviewed-by: Henry RobinsonTested-by: Henry Robinson --- M CMakeLists.txt M NOTICE.txt M be/CMakeLists.txt M be/src/common/config.h.in M be/src/kudu/security/init.cc M be/src/kudu/security/token_verifier.cc M bin/bootstrap_build.sh M cmake_modules/FindKerberos.cmake 8 files changed, 92 insertions(+), 18 deletions(-) Approvals: Henry Robinson: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/5717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Gerrit-PatchSet: 21 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build
Henry Robinson has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Add security library to build .. Patch Set 20: Code-Review+2 Verified+1 Rebase, carry +1 and +2. -- To view, visit http://gerrit.cloudera.org:8080/5717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Gerrit-PatchSet: 20 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8
Henry Robinson has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 .. Patch Set 16: Code-Review+2 Verified+1 Verified by build: https://jenkins.impala.io/job/gerrit-verify-dryrun/1051/ -- To view, visit http://gerrit.cloudera.org:8080/5716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Gerrit-PatchSet: 16 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8
Henry Robinson has submitted this change and it was merged. Change subject: IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 .. IMPALA-4669: [SECURITY] Import Kudu security library from kudu@314c9d8 The security library provides Kerberos and TLS facilities to the rpc library. Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Reviewed-on: http://gerrit.cloudera.org:8080/5716 Reviewed-by: Henry RobinsonTested-by: Henry Robinson --- M LICENSE.txt A be/src/kudu/security/CMakeLists.txt A be/src/kudu/security/ca/cert_management-test.cc A be/src/kudu/security/ca/cert_management.cc A be/src/kudu/security/ca/cert_management.h A be/src/kudu/security/cert-test.cc A be/src/kudu/security/cert.cc A be/src/kudu/security/cert.h A be/src/kudu/security/crypto-test.cc A be/src/kudu/security/crypto.cc A be/src/kudu/security/crypto.h A be/src/kudu/security/init.cc A be/src/kudu/security/init.h A be/src/kudu/security/kerberos_util.cc A be/src/kudu/security/kerberos_util.h A be/src/kudu/security/krb5_realm_override.cc A be/src/kudu/security/openssl_util.cc A be/src/kudu/security/openssl_util.h A be/src/kudu/security/openssl_util_bio.h A be/src/kudu/security/security-test-util.cc A be/src/kudu/security/security-test-util.h A be/src/kudu/security/simple_acl.cc A be/src/kudu/security/simple_acl.h A be/src/kudu/security/test/mini_kdc-test.cc A be/src/kudu/security/test/mini_kdc.cc A be/src/kudu/security/test/mini_kdc.h A be/src/kudu/security/test/test_certs.cc A be/src/kudu/security/test/test_certs.h A be/src/kudu/security/test/test_pass.cc A be/src/kudu/security/test/test_pass.h A be/src/kudu/security/tls_context.cc A be/src/kudu/security/tls_context.h A be/src/kudu/security/tls_handshake-test.cc A be/src/kudu/security/tls_handshake.cc A be/src/kudu/security/tls_handshake.h A be/src/kudu/security/tls_socket.cc A be/src/kudu/security/tls_socket.h A be/src/kudu/security/token-test.cc A be/src/kudu/security/token.proto A be/src/kudu/security/token_signer.cc A be/src/kudu/security/token_signer.h A be/src/kudu/security/token_signing_key.cc A be/src/kudu/security/token_signing_key.h A be/src/kudu/security/token_verifier.cc A be/src/kudu/security/token_verifier.h A be/src/kudu/security/x509_check_host.cc A be/src/kudu/security/x509_check_host.h M bin/rat_exclude_files.txt A cmake_modules/FindKerberos.cmake 49 files changed, 9,175 insertions(+), 0 deletions(-) Approvals: Henry Robinson: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/5716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I76daeead00f672aa468f5ab6de4d70eac2078cb2 Gerrit-PatchSet: 17 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Add security library to build .. Patch Set 19: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Gerrit-PatchSet: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-5477: Fix minidump-2-core tool in breakpad
Lars Volker has posted comments on this change. Change subject: IMPALA-5477: Fix minidump-2-core tool in breakpad .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iafe8cb59316bbec0a2c829f278a23a48238ad6e2 Gerrit-PatchSet: 3 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-5477: Fix minidump-2-core tool in breakpad
Lars Volker has submitted this change and it was merged. Change subject: IMPALA-5477: Fix minidump-2-core tool in breakpad .. IMPALA-5477: Fix minidump-2-core tool in breakpad The recent PPC port broke the minidump-2-core tool in the toolchain. This change undoes a seemingly erroneous #ifdef to fix the tool on non-PPC systems. To test this, I built a toolchain locally and made sure that cores created from minidumps with the tool can be opened by GDB. I didn't test the change on PPC. Change-Id: Iafe8cb59316bbec0a2c829f278a23a48238ad6e2 --- M buildall.sh A source/breakpad/breakpad-1b704857f1e78a864e6942e613457e55f1aecb60-patches/0003-fix-md2core.patch 2 files changed, 20 insertions(+), 1 deletion(-) Approvals: Lars Volker: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/7658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iafe8cb59316bbec0a2c829f278a23a48238ad6e2 Gerrit-PatchSet: 3 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-5477: Fix minidump-2-core tool in breakpad
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5477: Fix minidump-2-core tool in breakpad .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/7658/2/buildall.sh File buildall.sh: Line 273: BREAKPAD_VERSION=1b704857f1e78a864e6942e613457e55f1aecb60-p3 $SOURCE_DIR/source/breakpad/build.sh Can you move the old version into BUILD_HISTORICAL for consistency with elsewhere? It's not super-important but good to be consistent. -- To view, visit http://gerrit.cloudera.org:8080/7658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iafe8cb59316bbec0a2c829f278a23a48238ad6e2 Gerrit-PatchSet: 2 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Fold some lines
Laurel Hale has posted comments on this change. Change subject: [DOCS] Fold some lines .. Patch Set 1: Code-Review+1 PDF build looks good. -- To view, visit http://gerrit.cloudera.org:8080/7417 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I563a71ab9e59c691264c1f6a088e71f4722d3097 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John RussellGerrit-Reviewer: Ambreen Kazi Gerrit-Reviewer: Laurel Hale Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5769: Add periodic minidump cleanup
Lars Volker has posted comments on this change. Change subject: IMPALA-5769: Add periodic minidump cleanup .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7605/1/be/src/util/minidump.cc File be/src/util/minidump.cc: PS1, Line 113: CheckAndRemoveMinidumps > nit: Maybe we should call this "CheckAndRotateMinidumps" to be consistent w Done -- To view, visit http://gerrit.cloudera.org:8080/7605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie02ff2271412d814f84a4ff42ccbca51d91bf980 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5769: Add periodic minidump cleanup
Hello Sailesh Mukil, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7605 to look at the new patch set (#2). Change subject: IMPALA-5769: Add periodic minidump cleanup .. IMPALA-5769: Add periodic minidump cleanup Minidumps can be written by sending SIGUSR1 to our daemon processes. That way, an arbitrary number of minidump files can be created. This change adds minidump cleanup to the periodic log file cleanup to effectively bound the maximum number of minidumps we keep around. Change-Id: Ie02ff2271412d814f84a4ff42ccbca51d91bf980 --- M be/src/common/init.cc M be/src/util/minidump.cc M be/src/util/minidump.h M tests/custom_cluster/test_breakpad.py 4 files changed, 47 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/7605/2 -- To view, visit http://gerrit.cloudera.org:8080/7605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie02ff2271412d814f84a4ff42ccbca51d91bf980 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1055/ -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath VissapragadaGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath VissapragadaGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] Fix link to Hadoop ADLS page
Tim Armstrong has posted comments on this change. Change subject: Fix link to Hadoop ADLS page .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bcbf31735f398a879595846aeb1a6e1c7def9a2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John RussellGerrit-Reviewer: Laurel Hale Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1478: Improve error message when subquery is used in the ON clause
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-1478: Improve error message when subquery is used in the ON clause .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1054/ -- To view, visit http://gerrit.cloudera.org:8080/7588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Alex BehmGerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pranay Singh Gerrit-HasComments: No
[Impala-ASF-CR] Fix link to Hadoop ADLS page
Laurel Hale has posted comments on this change. Change subject: Fix link to Hadoop ADLS page .. Patch Set 1: Code-Review+1 I built the documentation and the link works perfectly. -- To view, visit http://gerrit.cloudera.org:8080/7672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bcbf31735f398a879595846aeb1a6e1c7def9a2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John RussellGerrit-Reviewer: Laurel Hale Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1478: Improve error message when subquery is used in the ON clause
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-1478: Improve error message when subquery is used in the ON clause .. Patch Set 5: Code-Review+2 +2'ing since the patch is simple. -- To view, visit http://gerrit.cloudera.org:8080/7588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0d1dc47987de7ea04402e1ead31d81cddf2f96f2 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Alex BehmGerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Pranay Singh Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4786: Clean up how ImpalaServers are created
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/7673 Change subject: IMPALA-4786: Clean up how ImpalaServers are created .. IMPALA-4786: Clean up how ImpalaServers are created ImpalaServer had to be created via an awkward CreateImpalaServer() method that took a lot of arguments. This patch refactors that code (in anticipation of some KRPC changes) to follow a more normal lifecycle: 1. ImpalaServer* server = new ImpalaServer(ExecEnv*) 2. RETURN_IF_ERROR(server->Init()) // for error-returning init operations 3. RETURN_IF_ERROR(server->Start()) 4. server->Join() Also add ExecEnv::Init(), and move calls to ExecEnv::StartServices() to ImpalaServer::StartServices(). This captures a dependency that KRPC will rely on - where initialization of both ExecEnv and ImpalaServer need to happen before services are started. This sets up a clean-up of InProcessImpalaServer, which is too heavy for the work that it does. That work is deferred to a follow-on patch. Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae --- M be/src/exprs/expr-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/scheduling/scheduler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/impalad-main.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M be/src/util/hdfs-util-test.cc 10 files changed, 108 insertions(+), 137 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/7673/1 -- To view, visit http://gerrit.cloudera.org:8080/7673 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson
[Impala-ASF-CR] IMPALA-5769: Add periodic minidump cleanup
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5769: Add periodic minidump cleanup .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/7605/1/be/src/util/minidump.cc File be/src/util/minidump.cc: PS1, Line 113: CheckAndRemoveMinidumps nit: Maybe we should call this "CheckAndRotateMinidumps" to be consistent with the other log rotation functions? Your call though, I'd be fine with this name too. -- To view, visit http://gerrit.cloudera.org:8080/7605 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie02ff2271412d814f84a4ff42ccbca51d91bf980 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Sailesh MukilGerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Joe McDonnell has posted comments on this change. Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. Patch Set 2: (1 comment) Fix long line and fix a backend test I had missed. http://gerrit.cloudera.org:8080/#/c/7597/2/be/src/exec/exec-node.cc File be/src/exec/exec-node.cc: Line 218:<< state->instance_mem_tracker()->LogUsage(MemTracker::UNLIMITED_DEPTH); > Long line Done -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7597 to look at the new patch set (#3). Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. IMPALA-5598: Fix excessive dumping in MemLimitExceeded ExecQueryFInstances RPC timeouts in stress tests were traced to excessive dumping in MemLimitExceeded() and LogUsage(). The QueryState::Init() hits the process memory limit, and this causes MemLimitExceeded to dump the process memory tracker. On the stress test, this involves dumping hundreds of queries and all the structures underneath. This is expensive and the contention between threads accessing these structures causes RPC timeouts. This adds the ability to the limit the recursive depth when dumping memory trackers. It also modifies the dumping in MemLimitExceeded() to have the following semantics: 1. The query memory tracker is always logged in full if it exists. 2. The process memory tracker is logged if the query memory tracker doesn't exist or if the process memory limit is being hit. The process memory tracker is limited to dumping only two levels of children. Other uses of LogUsage() are not limited (e.g. /memz and the query memory page on the web UI). A stress test run with the process memory tracker limited to two levels showed no RPC timeouts. Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 --- M be/src/exec/exec-node.cc M be/src/exec/hash-table-test.cc M be/src/runtime/bufferpool/reservation-tracker-test.cc M be/src/runtime/initial-reservations.cc M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/runtime/runtime-state.cc M be/src/service/impala-http-handler.cc M be/src/util/default-path-handlers.cc 9 files changed, 62 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/3 -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Fix link to Hadoop ADLS page
John Russell has uploaded a new change for review. http://gerrit.cloudera.org:8080/7672 Change subject: Fix link to Hadoop ADLS page .. Fix link to Hadoop ADLS page At the time I added the original link, the URL with /current2/ was correct. Now /current2/ no longer exists but /current3/ does. /current/ appears to be a stable alias that points to the most recent /currentN/ directory so I used that in the corrected link. Change-Id: I5bcbf31735f398a879595846aeb1a6e1c7def9a2 --- M docs/topics/impala_adls.xml 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/7672/1 -- To view, visit http://gerrit.cloudera.org:8080/7672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5bcbf31735f398a879595846aeb1a6e1c7def9a2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John Russell
[Impala-ASF-CR] IMPALA-4861: READ WRITE warning on CREATE TABLE LIKE PARQUET.
Pranay Singh has uploaded a new change for review. http://gerrit.cloudera.org:8080/7671 Change subject: IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. .. IMPALA-4861: READ_WRITE warning on CREATE TABLE LIKE PARQUET. Fix: If the source URI does not have write permissions we get a "READ WRITE warning" on CREATE TABLE LIKE PARQUET. During analyze() in the class CreateTableLikeFileStmt we should be checking only for "READ" permission for the source URI not for READ and WRITE as the source URI should be readable. Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab --- M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/7671/1 -- To view, visit http://gerrit.cloudera.org:8080/7671 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I56799c4da482fb634ce440f8764dd44234dc22ab Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pranay Singh
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 10: This failed in authorization/test_authorization.py::TestAuthorization::()::test_impersonation I don't think we've seen a failure there before so it may be related to the change. -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 10: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1050/ -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. Patch Set 2: Code-Review+1 (1 comment) Will let MJ take a look too. http://gerrit.cloudera.org:8080/#/c/7597/2/be/src/exec/exec-node.cc File be/src/exec/exec-node.cc: Line 218:<< state->instance_mem_tracker()->LogUsage(MemTracker::UNLIMITED_DEPTH); Long line -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Joe McDonnell has uploaded a new patch set (#2). Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. IMPALA-5598: Fix excessive dumping in MemLimitExceeded ExecQueryFInstances RPC timeouts in stress tests were traced to excessive dumping in MemLimitExceeded() and LogUsage(). The QueryState::Init() hits the process memory limit, and this causes MemLimitExceeded to dump the process memory tracker. On the stress test, this involves dumping hundreds of queries and all the structures underneath. This is expensive and the contention between threads accessing these structures causes RPC timeouts. This adds the ability to the limit the recursive depth when dumping memory trackers. It also modifies the dumping in MemLimitExceeded() to have the following semantics: 1. The query memory tracker is always logged in full if it exists. 2. The process memory tracker is logged if the query memory tracker doesn't exist or if the process memory limit is being hit. The process memory tracker is limited to dumping only two levels of children. Other uses of LogUsage() are not limited (e.g. /memz and the query memory page on the web UI). A stress test run with the process memory tracker limited to two levels showed no RPC timeouts. Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 --- M be/src/exec/exec-node.cc M be/src/exec/hash-table-test.cc M be/src/runtime/initial-reservations.cc M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/runtime/runtime-state.cc M be/src/service/impala-http-handler.cc M be/src/util/default-path-handlers.cc 8 files changed, 61 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/2 -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded
Joe McDonnell has posted comments on this change. Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded .. Patch Set 1: (3 comments) Rebased all the way. http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.cc File be/src/runtime/mem-tracker.cc: PS1, Line 341: (process_capacity < failed_allocation_size) > nit: unnecessary parens. Done PS1, Line 344: 1 > +1 I was wondering about this as well. The pool tracker is probably not goi Changed this to two levels. Stress tests show that this isn't a problem. I also added constants for the code to use. http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: PS1, Line 324: int max_recursive_levels = INT_MAX > Maybe we should make it a required argument to force new callsites to think Changed this to a required argument. -- To view, visit http://gerrit.cloudera.org:8080/7597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnellGerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5787: Dropped status in KuduTableSink::Send()
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-5787: Dropped status in KuduTableSink::Send() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7667/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS1, Line 241: // This can only fail if we set a col to an incorrect type, which would be a bug in : // planning, so we can DCHECK. > can you check with the kudu developers (e.g. on the Kudu dev slack channel) I checked with the Kudu team, and as long as the schema is what we want when the table is opened, it should be fine. If the schema changes in an incompatible way after the table is opened but before the writes are applied, the writes will fail gracefully at Appy(). However, there's a window of vulnerability between when we check the schema in the FE and when the table object is opened, so I explored this in detail, and there are a few cases to consider: - Column type changes: we don't allow altering existing column's types. - Column renamed: everything works as expected. - Column added: we don't allow adding primary keys, and non-nullable columns that are added must have a default, so everything works as expected. - Column dropped: this causes a crash, with or without this change. I filed IMPALA-5799 -- To view, visit http://gerrit.cloudera.org:8080/7667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4794: Grouping distinct agg plan robust to data skew
Tianyi Wang has posted comments on this change. Change subject: IMPALA-4794: Grouping distinct agg plan robust to data skew .. Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/7643/3//COMMIT_MSG Commit Message: Line 11: plan partitions data between phase-1 and phase-2 by the grouping exprs. > the grouping exprs Done Line 12: Under this strategy the data skewness on the grouping exprs directly > make this statement about skew a separate sentence Done Line 13: impacts performance. The new plan partitions data by both the grouping > by both the grouping and distinct agg exprs Done Line 14: exprs and distinct agg exprs, then adds one more aggregation and > Try to avoid descriptions like "supposed to be". We should test and underst Done Line 19: sufficient coverage. The pattern is that the distinct agg exprs are > the first exchange node Done -- To view, visit http://gerrit.cloudera.org:8080/7643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tianyi Wang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4794: Grouping distinct agg plan robust to data skew
Tianyi Wang has uploaded a new patch set (#4). Change subject: IMPALA-4794: Grouping distinct agg plan robust to data skew .. IMPALA-4794: Grouping distinct agg plan robust to data skew This patch changes the query plan for grouping distinct aggregations to be more robust to data skew in the grouping expressions. The existing plan partitions data between phase-1 and phase-2 by the grouping exprs. Under this strategy the data skewness on the grouping exprs directly impacts performance. The new plan partitions data by both the grouping exprs and distinct agg exprs, then adds one more aggregation and exchange node. The new plan is more robust to data skew but does more work than the old plan. Testing: Modified existing planner tests which already provide sufficient coverage. The pattern is that the distinct agg exprs are added to the first exchange node, followed by an additional merge agg and exchange node. Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 --- M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test M testdata/workloads/functional-planner/queries/PlannerTest/distinct.test M testdata/workloads/functional-planner/queries/PlannerTest/insert.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test 7 files changed, 215 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/7643/4 -- To view, visit http://gerrit.cloudera.org:8080/7643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1052/ -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Tim Armstrong has uploaded a new patch set (#5). Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() Plumbing statuses through TextConverter::CodegenWriteSlot(), which eventually propagate to the runtime profile. Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 --- M be/src/exec/hdfs-scanner.cc M be/src/exec/text-converter.cc M be/src/exec/text-converter.h 3 files changed, 53 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/7574/5 -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()
Tim Armstrong has posted comments on this change. Change subject: IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() .. Patch Set 3: Code-Review+2 One business day has elapsed and it looks like Lars' comments were addressed. Let's move forward. -- To view, visit http://gerrit.cloudera.org:8080/7574 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadkeGerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Dan Hecht has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5743: Support TLS version configuration for Thrift servers .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2422: Fix escaping in LIKE clause
Dan Hecht has posted comments on this change. Change subject: IMPALA-2422: Fix escaping in LIKE clause .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7660/2/be/src/exprs/like-predicate.cc File be/src/exprs/like-predicate.cc: Line 34: // A regex to match any regex pattern is equivalent to a substring search. isn't this comment referring to line 38? assuming so, it should be kept adjacent it it. -- To view, visit http://gerrit.cloudera.org:8080/7660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec0f5d99c855ea8d4fe1bcf28c05780ba315034b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4794: Grouping distinct agg plan robust to data skew
Tianyi Wang has posted comments on this change. Change subject: IMPALA-4794: Grouping distinct agg plan robust to data skew .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/7643/2/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: Line 859:* as its root. > update comment Done Line 864: // The phase-1 aggregation node is already in the child fragment. > move into previous line Done Line 916: phase2AggNode.setIntermediateTuple(); > Newlines should be used to visually cluster code that logically belongs tog Done Line 917: > You still need to do phase2AggNode.setIsPreagg(ctx_); I think that only works with grouping expr and is at line 922. -- To view, visit http://gerrit.cloudera.org:8080/7643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tianyi Wang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4794: Grouping distinct agg plan robust to data skew
Tianyi Wang has uploaded a new patch set (#3). Change subject: IMPALA-4794: Grouping distinct agg plan robust to data skew .. IMPALA-4794: Grouping distinct agg plan robust to data skew This patch changes the query plan for grouping distinct aggregations to be more robust to data skew in the grouping expressions. The existing plan partitions data between phase-1 and phase-2 by grouping expr and the data skewness on grouping expr directly impacts performance. The new plan partitions data by both grouping expr and distinct aggregation expr, then adds one more aggregation and exchange node. It is supposed to be faster with data skew but slower otherwise. Teting: Modified existing planner tests which already provide sufficient coverage. The pattern is that the distinct aggregation expr is added to exchange node, followed by an additional merge aggregation and exchange node. Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 --- M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test M testdata/workloads/functional-planner/queries/PlannerTest/distinct.test M testdata/workloads/functional-planner/queries/PlannerTest/insert.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test 7 files changed, 215 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/7643/3 -- To view, visit http://gerrit.cloudera.org:8080/7643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-4794: Grouping distinct agg plan robust to data skew
Alex Behm has posted comments on this change. Change subject: IMPALA-4794: Grouping distinct agg plan robust to data skew .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7643/2/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: Line 859:* as its root and 'node' is the second phase of the distinct aggregation. update comment Line 864: throws ImpalaException { move into previous line Line 916: Newlines should be used to visually cluster code that logically belongs together. Move this newline right before "DataPartition mergePartition;" - that's where we begin to add the merge agg node (in the preceding code separated by newline we added the phase2AggNode as an intermediate aggregation step) Line 917: phase2AggNode.unsetNeedsFinalize(); You still need to do phase2AggNode.setIsPreagg(ctx_); -- To view, visit http://gerrit.cloudera.org:8080/7643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7bdada0e328b555900c7b7ff8aabc8eb15ae8fa9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tianyi Wang Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5787: Dropped status in KuduTableSink::Send()
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5787: Dropped status in KuduTableSink::Send() .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7667/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS1, Line 241: // This can only fail if we set a col to an incorrect type, which would be a bug in : // planning, so we can DCHECK. can you check with the kudu developers (e.g. on the Kudu dev slack channel) that this is also safe in the context of concurrent schema changes? I'm 95% confident that this is the case, but it'd be good to check. -- To view, visit http://gerrit.cloudera.org:8080/7667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4669: [SECURITY] Add security library to build .. Patch Set 19: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1051/ -- To view, visit http://gerrit.cloudera.org:8080/5717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Gerrit-PatchSet: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build
Hello Impala Public Jenkins, Michael Ho, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5717 to look at the new patch set (#19). Change subject: IMPALA-4669: [SECURITY] Add security library to build .. IMPALA-4669: [SECURITY] Add security library to build * Minor compilation fix * Add krb5 as a non-toolchain dependency * Handle legacy versions of libkrb5.so by providing implementation of krb5_is_config_principal(). * Link against openssl from the toolchain if 1.0.0 or higher not found on build machine. * Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}. Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 --- M CMakeLists.txt M NOTICE.txt M be/CMakeLists.txt M be/src/common/config.h.in M be/src/kudu/security/init.cc M be/src/kudu/security/token_verifier.cc M bin/bootstrap_build.sh M cmake_modules/FindKerberos.cmake 8 files changed, 92 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/5717/19 -- To view, visit http://gerrit.cloudera.org:8080/5717 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14 Gerrit-PatchSet: 19 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-3931: arbitrary fixed-size uda intermediate types
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3931: arbitrary fixed-size uda intermediate types .. Patch Set 12: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/7526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ife90cf27989f98ffb5ef5c39f1e09ce92e8cb87c Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3548: Prune runtime filters based on query options in the FE .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7564 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila JegesGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5787: Dropped status in KuduTableSink::Send()
Thomas Tauber-Marshall has uploaded a new change for review. http://gerrit.cloudera.org:8080/7667 Change subject: IMPALA-5787: Dropped status in KuduTableSink::Send() .. IMPALA-5787: Dropped status in KuduTableSink::Send() KuduTableSink drops the Status returned from WriteKuduValue(), potentially hiding failures. There are only two possible ways WriteKuduValue() can fail: one of the KuduPartialRow::Set functions fails, which can only happen if the specified type is incorrect, which would be a bug in planning and is therefore DCHECK-able, or if TimestampValue::UtcToUnixTimeMicros() fails, which WriteKuduValue() DCHECKs against but also has a path to return the error in release builds. So similarly, this patch adds a DCHECK that the status is OK, and a path to return the error in release builds. It also adds WARN_UNUSED_RESULT to WriteKuduValue() to prevent similar bugs in the future. Testing: - Ran the Kudu tests. Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f --- M be/src/exec/kudu-table-sink.cc M be/src/exec/kudu-util.h 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/7667/1 -- To view, visit http://gerrit.cloudera.org:8080/7667 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer
Henry Robinson has posted comments on this change. Change subject: IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer .. Patch Set 5: (9 comments) This looks good! Just needs to merge with the recent ThriftServer changes, and consider adding a test where you set the number of concurrent clients and confirm that only that many can connect at once. http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/benchmarks/network-perf-benchmark.cc File be/src/benchmarks/network-perf-benchmark.cc: Line 206: impala::InitCommonRuntime(argc, argv, false, impala::TestInfo::BE_TEST); > I don't think network-perf-benchmark gets used much because it had bit rott This is fine - it's sufficient to keep it compiling for now. http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/catalog/catalogd-main.cc File be/src/catalog/catalogd-main.cc: Line 91: ThriftServer* server = new ThriftServer("CatalogService", processor, As you'll have seen, IMPALA-5696 switched over to a new ThriftServerBuilder idiom for constructing ThriftServer objects. I think what you'll need to do is remove the threaded() and thread_pool() methods, and just add a max_concurrent_cnxns(int) method. http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/rpc/TAcceptQueueServer.h File be/src/rpc/TAcceptQueueServer.h: Line 61: int32_t maxTasks, could you add a TODO to this class to suggest we determine which of these c'tors are actually needed? PS5, Line 114: notifies notified? Line 117: // The maximum number of running tasks allowed at a time. nit: put a new line before this one. http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/rpc/thrift-server.cc File be/src/rpc/thrift-server.cc: PS5, Line 409: stringstream key_prefix_ss; : key_prefix_ss << "impala.thrift-server." << name_; while you're here use Substitute("impala.thrift-server.$0", name_) ? http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/rpc/thrift-server.h File be/src/rpc/thrift-server.h: PS5, Line 176: conncurrent typo PS5, Line 177: until a thread becomes available nit: this makes it sound like the threads are pooled. Suggest 'connections will block until fewer than num_worker_threads_ threads are concurrently active' or something. http://gerrit.cloudera.org:8080/#/c/7061/5/be/src/transport/TSaslServerTransport.cpp File be/src/transport/TSaslServerTransport.cpp: PS5, Line 144: TThreadedServer and : // TThreadPoolServer remove these two? -- To view, visit http://gerrit.cloudera.org:8080/7061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John ShermanGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5708: Test failure with invalid exec summary
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-5708: Test failure with invalid exec summary .. Patch Set 3: Code-Review+2 Carrying forward -- To view, visit http://gerrit.cloudera.org:8080/7627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id52ac62da2b01f9e163e97cbe4590f8db6b663d2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5708: Test failure with invalid exec summary
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5708: Test failure with invalid exec summary .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id52ac62da2b01f9e163e97cbe4590f8db6b663d2 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5764: Allow overriding packaged components
Jim Apple has posted comments on this change. Change subject: IMPALA-5764: Allow overriding packaged components .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7581/2/bin/impala-config.sh File bin/impala-config.sh: Line 180: export DOWNLOAD_PACKAGED_COMPONENTS=${DOWNLOAD_PACKAGED_COMPONENTS-"$NO_THIRDPARTY"} > Technically this "breaks" the C6 build, but because the cdh6.x version of I This is the code review tool for Apache Impala. It is fine to add features to Impala to help other software that works with Impala, including proprietary products. However, please try to explain enough in your discussion here so that a person who does not work at Cloudera can help review your code and understand all the jargon like "pushed from CDH5" and "cauldron". -- To view, visit http://gerrit.cloudera.org:8080/7581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95c2662e6f62adc924cc5de7a371202126046545 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach AmsdenGerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5776: Write partial tuple to the correct mempool
Alex Behm has posted comments on this change. Change subject: IMPALA-5776: Write partial tuple to the correct mempool .. Patch Set 3: (7 comments) I like the new solution, it's easier to understand. http://gerrit.cloudera.org:8080/#/c/7639/2/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: Line 269: while (true) { > It's used on line 277. Are you suggesting to remove it from the signature o Oh I missed that sorry. Ok to leave. http://gerrit.cloudera.org:8080/#/c/7639/3/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: Line 804: // Copy over all materialized slots in the partial tuple. Only describes what the code does. How about something like: // Deep copy to simplify memory ownership. Line 808: // prevent the accumulation of variable length data in it. We also recreate the The partial tuple isn't re-created here http://gerrit.cloudera.org:8080/#/c/7639/3/be/src/exec/hdfs-text-scanner.h File be/src/exec/hdfs-text-scanner.h: Line 186: /// Utility function to write out 'num_fields' to 'tuple_'. This is used to parse Comment is weird, I suggest rephrasing to something like: Utility function to parse 'num_fields' and materialize the resulting slots into 'partial_tuple_'. The data of var-len fields is copied into 'partial_tuple_pool_'. Line 227: /// Mem pool for the partial tuple. Mention that it does not hold tuple data of returned batches because the data is always deep-copied into the output batch Line 230: /// Memory to store partial tuples split across buffers. Memory comes from Mention that this tuple is always deep copied into the output batch Line 231: /// partial_tuple_pool_. There is only one tuple allocated for this object and reused The 2nd sentence does not seem to apply anymore because we realloc the partial_tuple_ -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras BobrovytskyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5764: Allow overriding packaged components
Zach Amsden has posted comments on this change. Change subject: IMPALA-5764: Allow overriding packaged components .. Patch Set 2: (4 comments) A couple of minor nits. Also, let me make sure I didn't accidentally introduce typos during edits and that the script and data load still work. http://gerrit.cloudera.org:8080/#/c/7581/2/README.md File README.md: Line 69: | PACKAGED_COMPONENTS_NAME | "cdh" | Identifier used to uniquify paths for potentially incompatible component builds. | This doesn't currently match the "cdh5" label used in the shell script. http://gerrit.cloudera.org:8080/#/c/7581/2/bin/impala-config.sh File bin/impala-config.sh: Line 180: export DOWNLOAD_PACKAGED_COMPONENTS=${DOWNLOAD_PACKAGED_COMPONENTS-"$NO_THIRDPARTY"} Technically this "breaks" the C6 build, but because the cdh6.x version of Impala is pushed right now from CDH5 with a thirdparty directory, this is still working. Will need to update the cauldron build to fix this. Line 361: export PACKAGED_COMPONENTS_HOME="$IMPALA_TOOLCHAIN/$PACKAGED_COMPONENTS_NAME" This changes the download directory for the CDH pieces of the toolchain; I presume that is okay to do, might result in duplicate copies but nothing should break because of this. Line 382: export MINI_DFS_BASE_DATA_DIR="$IMPALA_HOME/${PACKAGED_COMPONENTS_NAME}-hdfs-data" This forces a data reload - unless there is any objection, I'm not going to try to change that. -- To view, visit http://gerrit.cloudera.org:8080/7581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95c2662e6f62adc924cc5de7a371202126046545 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach AmsdenGerrit-Reviewer: David Knupp Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Tianyi Wang has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 10: > (2 comments) > > Looks good. If you fix the last couple of nits and rebase I can > start the merge. Done -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1050/ -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7642 to look at the new patch set (#9). Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. IMPALA-5327: Handle return of JNI GetStringUTFChar GetStringUTFChars may return NULL or throw exception and it is not handled in two places. This patch checks the return value/exception, logs if there is error and return error to the caller. A helper class JniUtfCharGuard is added to jni-util.h/cc to manage the lifetime. Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 --- M be/src/common/status.h M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/logging-support.cc 4 files changed, 68 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/7642/9 -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5327: Handle return of JNI GetStringUTFChar
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5327: Handle return of JNI GetStringUTFChar .. Patch Set 8: Code-Review+2 (2 comments) Looks good. If you fix the last couple of nits and rebase I can start the merge. http://gerrit.cloudera.org:8080/#/c/7642/8/be/src/util/jni-util.h File be/src/util/jni-util.h: Line 164: DISALLOW_COPY_AND_ASSIGN(JniUtfCharGuard); Nit: move it inside 'private'. http://gerrit.cloudera.org:8080/#/c/7642/8/be/src/util/logging-support.cc File be/src/util/logging-support.cc: Line 53: str = ""; Nit: seems simpler to just initialise it to point to the empty string constant like it was before. -- To view, visit http://gerrit.cloudera.org:8080/7642 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47eed045f21c6ed3a8decf422ce8429fc66c4322 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tianyi WangGerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5796: CTAS for Kudu fails with expr rewrite
Thomas Tauber-Marshall has uploaded a new change for review. http://gerrit.cloudera.org:8080/7666 Change subject: IMPALA-5796: CTAS for Kudu fails with expr rewrite .. IMPALA-5796: CTAS for Kudu fails with expr rewrite When an expr rewrite occurs, we reanalyze the statement. Some state that is set in TableDef::analyze() wasn't being reset() first, causing a failure during reanalysis. This patch adds TableDef::reset(), which clears the TableDef state that is set during analyze(). Testing: - Added a regression test in AnalyzeDDLTest Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 4 files changed, 32 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/66/7666/1 -- To view, visit http://gerrit.cloudera.org:8080/7666 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-2615: support [[nodiscard]] on Status
Jim Apple has posted comments on this change. Change subject: IMPALA-2615: support [[nodiscard]] on Status .. Patch Set 9: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I972543af2e9f98b12dcbb5479b4c1a7d53952197 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3931: arbitrary fixed-size uda intermediate types
Tim Armstrong has posted comments on this change. Change subject: IMPALA-3931: arbitrary fixed-size uda intermediate types .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/7526/8/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: PS8, Line 467: // Represent this as an array of bytes. : return ArrayType::get(GetType(TYPE_TINYINT), type.len); > IMHO, this change in behavior makes GetType() an error prone interface. Giv I think the relationship is analogous to the existing cases: int64 <==> BIGINT StringValue<==> STRING TimestampValue <==> TIMESTAMP int128 <==> DECIMAL(38,7) int8[n]<==> FIXED_UDA_INTERMEDIATE[n] It's already a bug if you call this with TYPE_STRING when constructing an IR function argument since it returns StringValue (we should rename string_val_type and timestamp_val_type for sure though). http://gerrit.cloudera.org:8080/#/c/7526/11/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: PS11, Line 1540: bitcast { i64, i8* }* %dst_lowered_ptr to %"struct.impala_udf: > This seems unnecessary now but I guess DCE will eliminate it. Yeah, DCE will get it. I think it's simpler than modifying the codegen interfaces to avoid emitting the instruction in this special case. http://gerrit.cloudera.org:8080/#/c/7526/8/be/src/exprs/agg-fn-evaluator.cc File be/src/exprs/agg-fn-evaluator.cc: PS8, Line 478: TYPE_FIXED_UDA_INTERMEDIATE: > Is TYPE_FIXED_UDA_INTERMEIDATE limited to built-in UDA only ? stddev already uses it internally and there doesn't seem to have been anything preventing declaration of a UDA using CHAR(n). Codegen is disabled though. On master I can declare a UDA with a CHAR intermediate if I load the UDAs from this patch: $ MAKE_CMD=ninja ./testdata/bin/copy-udfs-udas.sh -build $ git checkout origin/master $ git rev-parse HEAD df9ecdc45a34b176f2ee6eda8e12d0195fba9ae7 [localhost:21000] > create aggregate function udasum(int) RETURNS int INTERMEDIATE CHAR(10) LOCATION '/test-warehouse/libTestUdas.so' UPDATE_FN='AggCharIntermediateUpdate' INIT_FN='AggCharIntermediateInit' MERGE_FN='AggCharIntermediateMerge' FINALIZE_FN='AggCharIntermediateFinalize'; Query: create aggregate function udasum(int) RETURNS int INTERMEDIATE CHAR(10) LOCATION '/test-warehouse/libTestUdas.so' UPDATE_FN='AggCharIntermediateUpdate' INIT_FN='AggCharIntermediateInit' MERGE_FN='AggCharIntermediateMerge' FINALIZE_FN='AggCharIntermediateFinalize' Fetched 0 row(s) in 0.17s [localhost:21000] > select year, month, day, udasum(int_col), sum(int_col) from functional.alltypesagg group by year, month, day; Query: select year, month, day, udasum(int_col), sum(int_col) from functional.alltypesagg group by year, month, day Query submitted at: 2017-08-14 10:54:10 (Coordinator: http://tarmstrong-box:25000) Query progress can be monitored at: http://tarmstrong-box:25000/query_plan?query_id=3546da729d693928:a81fcd1b Socket error 104: Connection reset by peer [Not connected] > Goodbye tarmstrong So I guess it's allowed but broken. Arguably it might be better to disallow it instead of fixing it. -- To view, visit http://gerrit.cloudera.org:8080/7526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ife90cf27989f98ffb5ef5c39f1e09ce92e8cb87c Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3931: arbitrary fixed-size uda intermediate types
Hello Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7526 to look at the new patch set (#12). Change subject: IMPALA-3931: arbitrary fixed-size uda intermediate types .. IMPALA-3931: arbitrary fixed-size uda intermediate types Make many builtin aggregate functions use fixed-length intermediate types: * avg() * ndv() * stddev(), variance(), etc * distinctpc(), distinctpcsa() sample(), appx_median(), histogram() and group_concat() actually allocate var-len data so aren't changed. This has some major benefits: * Spill-to-disk works properly with these aggregations. * Aggregations are more efficient because there is one less pointer indirection. * Aggregations use less memory, because we don't need an extra 12-byte StringValue for the indirection. Adds a special-purpose internal type FIXED_UDA_INTERMEDIATE. The type is represented in the same way as CHAR - a fixed-size array of bytes, stored inline in tuples. However, it is not user-visible and does not support CHAR semantics, i.e. users can't declare tables, functions, etc with the type. The pointer and length is passed into aggregate functions wrapped in a StringVal. Updates some internal codegen functions to work better with the new type. E.g. store values directly into the result tuple instead of via an intermediate stack allocation. Testing: This change only affects builtin aggregate functions, for which we have test coverage already. If we were to allow wider use of this type, it would need further testing. Added an analyzer test to ensure we can't use the type for UDAs. Added a regression test for spilling avg(). Added a regression test for UDA with CHAR intermediate hitting DCHECK. Perf: Ran TPC-H locally. TPC-H Q17, which has a high-cardinality AVG(), improved dramatically. +--+---+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | +--+---+-++++ | TPCH(60) | parquet / none / none | 18.44 | -17.54%| 11.92 | -5.34% | +--+---+-++++ +--+--+---++-++---++-+---+ | Workload | Query| File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | +--+--+---++-++---++-+---+ | TPCH(60) | TPCH-Q12 | parquet / none / none | 18.40 | 17.64 | +4.32% | 0.77% | 1.09%| 1 | 5 | | TPCH(60) | TPCH-Q22 | parquet / none / none | 7.07 | 6.90| +2.36% | 0.28% | 0.30%| 1 | 5 | | TPCH(60) | TPCH-Q3 | parquet / none / none | 12.37 | 12.11 | +2.10% | 0.18% | 0.15%| 1 | 5 | | TPCH(60) | TPCH-Q7 | parquet / none / none | 42.48 | 42.09 | +0.93% | 2.45% | 0.80%| 1 | 5 | | TPCH(60) | TPCH-Q6 | parquet / none / none | 3.18 | 3.15| +0.89% | 0.67% | 0.76%| 1 | 5 | | TPCH(60) | TPCH-Q19 | parquet / none / none | 7.24 | 7.20| +0.50% | 0.95% | 0.67%| 1 | 5 | | TPCH(60) | TPCH-Q10 | parquet / none / none | 13.37 | 13.30 | +0.50% | 0.48% | 1.39%| 1 | 5 | | TPCH(60) | TPCH-Q5 | parquet / none / none | 7.47 | 7.44| +0.36% | 0.58% | 0.54%| 1 | 5 | | TPCH(60) | TPCH-Q11 | parquet / none / none | 2.03 | 2.02| +0.06% | 0.26% | 1.95%| 1 | 5 | | TPCH(60) | TPCH-Q4 | parquet / none / none | 5.48 | 5.50| -0.27% | 0.62% | 1.12%| 1 | 5 | | TPCH(60) | TPCH-Q13 | parquet / none / none | 22.11 | 22.18 | -0.31% | 0.18% | 0.55%| 1 | 5 | | TPCH(60) | TPCH-Q15 | parquet / none / none | 8.45 | 8.48| -0.32% | 0.40% | 0.47%| 1 | 5 | | TPCH(60) | TPCH-Q9 | parquet / none / none | 33.39 | 33.66 | -0.81% | 0.75% | 0.59%| 1 | 5 | | TPCH(60) | TPCH-Q21 | parquet / none / none | 71.34 | 72.07 | -1.01% | 1.84% | 1.79%| 1 | 5 | | TPCH(60) | TPCH-Q14 | parquet / none / none | 5.93 | 6.00| -1.07% | 0.15% | 0.69%| 1 | 5 | | TPCH(60) | TPCH-Q20 | parquet / none / none | 5.72 | 5.79| -1.09% | 0.59% | 0.51%| 1 | 5 | | TPCH(60) | TPCH-Q18 | parquet / none / none | 45.42 |
[Impala-ASF-CR] IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer
Henry Robinson has posted comments on this change. Change subject: IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer .. Patch Set 5: Sorry for the delay, John! I'll try to have comments later today. There's no particular etiquette - you're free to post a new patch if the reviewers (i.e. me!) are taking their time over comments... -- To view, visit http://gerrit.cloudera.org:8080/7061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John ShermanGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3931: arbitrary fixed-size uda intermediate types
Michael Ho has posted comments on this change. Change subject: IMPALA-3931: arbitrary fixed-size uda intermediate types .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/7526/8/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: PS8, Line 467: // Represent this as an array of bytes. : return ArrayType::get(GetType(TYPE_TINYINT), type.len); > I think this makes sense so long as we make it clear that the function retu IMHO, this change in behavior makes GetType() an error prone interface. Given there is no clear guideline on which one to use, it's likely that one may call this function when generating a signature for a IR function or any call sites which use both GetType() and GetUnloweredType() may result in surprises. It seems safer to have a separate function for the internal representation. We should also document clearly how FIXED_UDA_INTERMEDIATE is represented in IR. For instance, we are mostly exposing it as a StringVal but when is it appropriate to represent it as an ArrayType. http://gerrit.cloudera.org:8080/#/c/7526/11/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: PS11, Line 1540: bitcast { i64, i8* }* %dst_lowered_ptr to %"struct.impala_udf: This seems unnecessary now but I guess DCE will eliminate it. http://gerrit.cloudera.org:8080/#/c/7526/8/be/src/exprs/agg-fn-evaluator.cc File be/src/exprs/agg-fn-evaluator.cc: PS8, Line 478: TYPE_FIXED_UDA_INTERMEDIATE: > A DCHECK doesn't seem appropriate since a UDA could set the incorrect thing Is TYPE_FIXED_UDA_INTERMEIDATE limited to built-in UDA only ? Btw, did we not support TYPE_CHAR as intermediate type before ? Is it a change in behavior ? -- To view, visit http://gerrit.cloudera.org:8080/7526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ife90cf27989f98ffb5ef5c39f1e09ce92e8cb87c Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer
John Sherman has posted comments on this change. Change subject: IMPALA-5394: Change ThriftServer() to always use TAcceptQueueServer .. Patch Set 5: I don't quite know the correct etiquette, should I fix the merge conflict with recent changes or wait for review comments on my current change set? -- To view, visit http://gerrit.cloudera.org:8080/7061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: John ShermanGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: John Sherman Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[native-toolchain-CR] Bump Kudu version to b198ed8
Thomas Tauber-Marshall has submitted this change and it was merged. Change subject: Bump Kudu version to b198ed8 .. Bump Kudu version to b198ed8 Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb --- M buildall.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Matthew Jacobs: Looks good to me, approved Thomas Tauber-Marshall: Verified -- To view, visit http://gerrit.cloudera.org:8080/7665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb Gerrit-PatchSet: 1 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall
[native-toolchain-CR] Bump Kudu version to b198ed8
Matthew Jacobs has posted comments on this change. Change subject: Bump Kudu version to b198ed8 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb Gerrit-PatchSet: 1 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Matthew Jacobs Gerrit-HasComments: No
[native-toolchain-CR] Bump Kudu version to b198ed8
Thomas Tauber-Marshall has posted comments on this change. Change subject: Bump Kudu version to b198ed8 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb Gerrit-PatchSet: 1 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-MarshallGerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4737: Prevent SIGUSR1 from killing daemons when minidumps are disabled
John Sherman has posted comments on this change. Change subject: IMPALA-4737: Prevent SIGUSR1 from killing daemons when minidumps are disabled .. Patch Set 2: No, I do not feel strongly about this. > > Should any startup scripts be modified to trap '' SIGUSR1 before > > launching the daemons since there is still technically a race > from > > daemon startup to when the mini-dump signal handler is > registered? > > (Though from what I can tell the typical usage of the SIGUSR1 > > mini-dump handler would very likely never encounter this race, so > > maybe not worth fixing?) > > Yes, that race exists, I don't think it's worth fixing though. > Human operators should not hit it. Automated scripts that regularly > trigger minidumps sending SIGUSR1 could hit this, but I think we > should not encourage those anyways. Let me know if you feel > strongly about this. -- To view, visit http://gerrit.cloudera.org:8080/7631 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I13d866a2eec832500131954a7f693c33585ea51e Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: John Sherman Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[native-toolchain-CR] Bump Kudu version to b198ed8
Thomas Tauber-Marshall has uploaded a new change for review. http://gerrit.cloudera.org:8080/7665 Change subject: Bump Kudu version to b198ed8 .. Bump Kudu version to b198ed8 Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb --- M buildall.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/65/7665/1 -- To view, visit http://gerrit.cloudera.org:8080/7665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia62deb0e61f06de4ce4e95476b85988e1e2754bb Gerrit-PatchSet: 1 Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-5715: (mitigation only) defer destruction of MemTrackers
Dan Hecht has posted comments on this change. Change subject: IMPALA-5715: (mitigation only) defer destruction of MemTrackers .. Patch Set 6: > I wrote up a short page on the wiki per Henry's suggestion: > https://cwiki.apache.org/confluence/display/IMPALA/Resource+Management+Best+Practices+in+Impala Thanks, that page looks good. -- To view, visit http://gerrit.cloudera.org:8080/7492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I205abb0076d1ffd08cb93c0f1671c8b81e7fba0f Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim ArmstrongGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No