[Impala-ASF-CR] IMPALA-7640: RENAME on managed Kudu table should rename Kudu table
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12179 ) Change subject: IMPALA-7640: RENAME on managed Kudu table should rename Kudu table .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77e7583ce93cba8f6e743c4bedd9900ae1fae081 Gerrit-Change-Number: 12179 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Sat, 02 Feb 2019 07:21:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8101: Thrift 11 and ext-data-source compilation are always run
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12290 )
Change subject: IMPALA-8101: Thrift 11 and ext-data-source compilation are
always run
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12290/1/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12290/1/common/thrift/CMakeLists.txt@121
PS1, Line 121: COMMAND ${THRIFT_COMPILER} ${JAVA_EXT_DS_ARGS}
${THRIFT_FILE} &&
> Is it possible to remove the file when cleaning the build, e.g. when runnin
Nevermind, saw the commit message.
--
To view, visit http://gerrit.cloudera.org:8080/12290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52520e4b099c7bac5d088b4ba5d8a335495f727d
Gerrit-Change-Number: 12290
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Sat, 02 Feb 2019 07:13:43 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8101: Thrift 11 and ext-data-source compilation are always run
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12290 )
Change subject: IMPALA-8101: Thrift 11 and ext-data-source compilation are
always run
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12290/1/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12290/1/common/thrift/CMakeLists.txt@121
PS1, Line 121: COMMAND ${THRIFT_COMPILER} ${JAVA_EXT_DS_ARGS}
${THRIFT_FILE} &&
Is it possible to remove the file when cleaning the build, e.g. when running
clean.sh?
--
To view, visit http://gerrit.cloudera.org:8080/12290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52520e4b099c7bac5d088b4ba5d8a335495f727d
Gerrit-Change-Number: 12290
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Sat, 02 Feb 2019 07:13:16 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8129: Don't test exact value of ExchangeScanRatio on S3 and EC
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12288 ) Change subject: IMPALA-8129: Don't test exact value of ExchangeScanRatio on S3 and EC .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/12288/4/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12288/4/tests/query_test/test_observability.py@396 PS4, Line 396: @SkipIfNotHdfsMinicluster.tuned_for_minicluster > Nit: This is triple conditional is hard to read. Skip if it is not an HDFS Just saw this comment, apologies for missing it during the initial round of reviews. I agree, these are somewhat hard to decipher, but I found over time they become more clear, and it's consistent with what we usually do. Maybe you can think of ways to make them more readable in general? -- To view, visit http://gerrit.cloudera.org:8080/12288 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6cf58113e092d43f5444120040aa49f90cdb91fb Gerrit-Change-Number: 12288 Gerrit-PatchSet: 4 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Sat, 02 Feb 2019 06:53:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12339 ) Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Feb 2019 06:44:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 8 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Sat, 02 Feb 2019 06:09:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5872: Testcase builder for query planner
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/12221 )
Change subject: IMPALA-5872: Testcase builder for query planner
..
Patch Set 5:
(8 comments)
Reviewed the latest changes. Looking good. Mostly minor questions and comments
at this point.
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1178
PS5, Line 1178: Preconditions.checkState(whereSubQueries.size() == 1,
"Multiple subqueries not " +
This is a Preconditions which throws an unchecked exception. The structure of a
query is something the user specifies. Should this check be done during
analysis, throwing an AnalysisException?
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@288
PS5, Line 288: public CatalogServiceCatalog(boolean loadInBackground, int
numLoadingThreads,
Add Javadoc here that was removed from Catalog?
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1307
PS5, Line 1307: public Table addTable(Db db, Table table) {
Javadoc that explains how this is meant to be used?
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1142
PS5, Line 1142: "TotalNodes %s, Local Ranges %s",
tbl_.getFullName(), totalNodes,
%d for integers
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1151
PS5, Line 1151: // In debug mode, we assume that every scan range
is local to some node.
Not clear how we get here in debug mode.
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@341
PS5, Line 341: public void testCopyTestCasePrivileges() throws
ImpalaException {
Thanks for adding these tests!
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@3179
PS5, Line 3179: e.printStackTrace();
Remove prior to push to master?
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
File fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java:
http://gerrit.cloudera.org:8080/#/c/12221/5/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java@81
PS5, Line 81: Path derbyPath =
Paths.get(System.getProperty("java.io.tmpdir"),
I believe there is a cleaner way to get a temp directory. See
https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#createTempDirectory%28java.nio.file.Path,%20java.lang.String,%20java.nio.file.attribute.FileAttribute...%29
Also, to be nice, the directory should be deleted on exit.
--
To view, visit http://gerrit.cloudera.org:8080/12221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52
Gerrit-Change-Number: 12221
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Balazs Jeszenszky
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Sat, 02 Feb 2019 03:42:57 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/12334 )
Change subject: IMPALA-8150: Fix buggy
AuditingTest::TestAccessEventsOnAuthFailure
..
Patch Set 4:
(2 comments)
Thanks for cleaning this up, and for fixing the "polarity" of the assertEquals
statements. A few minor comments.
http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java:
http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java@83
PS4, Line 83: if (sentryConfig_.getConfigFile() != null) {
You've found that something prevents the file name from ever being ""? Or, that
we want to fail the load in this case?
http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:
http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@962
PS4, Line 962: "Authorization is enabled but the server name is null
or empty. Set the " +
Error message says null or empty, but the code was changed to just check null...
--
To view, visit http://gerrit.cloudera.org:8080/12334
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95
Gerrit-Change-Number: 12334
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Sat, 02 Feb 2019 03:19:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12339 ) Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3699/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Feb 2019 02:47:32 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12339 ) Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1969/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Feb 2019 02:44:50 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12339 ) Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1968/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Feb 2019 02:35:13 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12339 to look at the new patch set (#2). Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x The new commit in Impala-lzo/master breaks the builds of Impala-2.x. We should depend on a dedicated branch for 2.x. Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 --- M bin/bootstrap_system.sh 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/12339/2 -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR](2.x) IMPALA-8155: Modify bootstrap system.sh to bind Impala-lzo/2.x
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12339 ) Change subject: IMPALA-8155: Modify bootstrap_system.sh to bind Impala-lzo/2.x .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12339/1/bin/bootstrap_system.sh File bin/bootstrap_system.sh: http://gerrit.cloudera.org:8080/#/c/12339/1/bin/bootstrap_system.sh@209 PS1, Line 209: git clone https://github.com/cloudera/impala-lzo.git --branch 2.x --single-branch "$IMPALA_LZO_HOME" line too long (102 > 90) -- To view, visit http://gerrit.cloudera.org:8080/12339 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I67591c7cfc4bede5c096a49beb57da34bb697338 Gerrit-Change-Number: 12339 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Feb 2019 01:55:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7565: Set TAcceptQueueServer connection setup pool to be multi-threaded by default
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12249 ) Change subject: IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 Gerrit-Change-Number: 12249 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Sat, 02 Feb 2019 01:32:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7565: Set TAcceptQueueServer connection setup pool to be multi-threaded by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12249 ) Change subject: IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1967/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 Gerrit-Change-Number: 12249 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Sat, 02 Feb 2019 01:00:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 7: The failure is not related. Builds against other jira patches(1963, 1964) has the same failure. -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 7 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Sat, 02 Feb 2019 00:23:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7565: Set TAcceptQueueServer connection setup pool to be multi-threaded by default
Hello Michael Ho, Thomas Marshall, Zoram Thanga, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12249 to look at the new patch set (#2). Change subject: IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default .. IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default Bumping up the thread count for the threads that process the post-accept, pre-setup connection queue to 2 in order to minimize the chances of a single client holding up others if the thread gets stuck in that step. This patch also un-hides the advanced startup flag 'accepted_cnxn_setup_thread_pool_size'. Testing: - Ran exhaustive tests with a thread pool of 10. - Scanned manually through the code and parts of thrift lib to make sure the APIs are used in a thread safe manner. - Rapidly executed openssl connect-disconnect on the impalad's hs2 server port on a thread sanitizer build. No data races were flagged by the thread sanitizer. - Ran a stress test on an 8 node secure cluster with 100 concurrent streams executing the tpch workload with a scale of 5g and a connection_setup_pool of 100. Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 --- M be/src/rpc/TAcceptQueueServer.cpp M be/src/transport/TSaslServerTransport.cpp 2 files changed, 5 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/12249/2 -- To view, visit http://gerrit.cloudera.org:8080/12249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 Gerrit-Change-Number: 12249 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12334 ) Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure .. Patch Set 4: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1966/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 23:15:16 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. IMPALA-7144: Re-enable TestDescribeTableResults This patch makes the TestDescribeTableResults more robust by only comparing the information that the authorization cares about instead of comparing all output in DESCRIBE. This change will avoid any unnecessary changes to AuthorizationTest if HMS updates the DESCRIBE output. The test is also updated to support standalone execution without relying on other tests be executed first since it can cause the test to be flaky especially if the tests in AuthorizationTest are executed in parallel. Testing: - Ran all FE tests Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Reviewed-on: http://gerrit.cloudera.org:8080/10643 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/12331 Reviewed-by: Fredy Wijaya --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 1 file changed, 174 insertions(+), 170 deletions(-) Approvals: Impala Public Jenkins: Verified Fredy Wijaya: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: merged Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 7: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1965/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 7 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 01 Feb 2019 23:10:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8092: Add an admission controller debug page
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12244 ) Change subject: IMPALA-8092: Add an admission controller debug page .. IMPALA-8092: Add an admission controller debug page This patch adds a new debug page "admission" that provides the following details about resource pools: - Pool configuration - Relevant pool stats - Queued Queries in order of being queued (local to the coordinator) - Running Queries (local to this coordinator) - Histogram of the distribution of peak memory used by queries admitted to the pool The aforementioned details can also be viewed for a single resource pool using a search string and are also available as a JSON object from the same http endpoint. Testing: - Added a test that checks if the admission debug page loads correctly and checks if the reset informational stats http end point works as expected. - Did manual stress testing by running the e2e tests and constantly fetching the admission debug page. Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Reviewed-on: http://gerrit.cloudera.org:8080/12244 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/catalog/catalog-server.cc M be/src/rpc/rpc-trace.cc M be/src/runtime/coordinator.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/service/impala-http-handler.cc M be/src/service/impala-http-handler.h M be/src/statestore/statestore.cc M be/src/util/default-path-handlers.cc M be/src/util/logging-support.cc M be/src/util/metrics.cc M be/src/util/thread.cc M be/src/util/webserver-test.cc M be/src/util/webserver.h M tests/webserver/test_web_pages.py A www/admission_controller.tmpl M www/common-header.tmpl 17 files changed, 787 insertions(+), 38 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Gerrit-Change-Number: 12244 Gerrit-PatchSet: 8 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8092: Add an admission controller debug page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12244 ) Change subject: IMPALA-8092: Add an admission controller debug page .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Gerrit-Change-Number: 12244 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 22:52:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12334 ) Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure .. Patch Set 4: Rebased to fix gerrit-code-review-checks failure. -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 22:46:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12334 ) Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure .. IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure I was able to reproduce this issue by running AuditingTest individually. Running all tests did not seem to reproduce this issue, which may be due to the test depends on the state from other tests. Looking at the code, the test was poorly written mainly because of two reasons: - The Sentry config was set to an empty string, which is not a valid config file. - Calling AuthorizationConfig.validateConfig() was missing. I also fixed a bug in AuthorizationConfig.validateConfig(), which skipped the Sentry config validation when the Sentry config file path is empty. As a result, I updated other tests that considered empty Sentry config file path to be valid. Testing: - Ran AuditingTest individually - Ran all FE tests Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 3 files changed, 25 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/12334/4 -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@13 PS6, Line 13: Add TimeSeriesCounter prefix checks in test_observability tests. > nit: trailing whitespace Done http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@19 PS6, Line 19: . . . > I'm not sure whether these ... in the commit message might cause issues for I used 3 single dots. To be safe, I will add space between them http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py@404 PS6, Line 404: assert " MemoryUsage" not in profile > I don't think you need a regex here but you could just do "assert pattern i Done -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 7 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Fri, 01 Feb 2019 22:30:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 20:52:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12334 ) Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1963/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 20:08:54 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "IMPALA-8146: Remove make {debug,release,asan}.sh"
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12335 )
Change subject: Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
..
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1962/ : Initial code
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun
to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 01 Feb 2019 20:14:52 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8148: Misc. FE code cleanup
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12317 ) Change subject: IMPALA-8148: Misc. FE code cleanup .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/1964/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/12317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e2f2e79f36e804fd592b6fe77f0554c5ca803eb Gerrit-Change-Number: 12317 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 20:13:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7928: Consistent remote read scheduling
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12037 ) Change subject: IMPALA-7928: Consistent remote read scheduling .. Patch Set 5: (42 comments) http://gerrit.cloudera.org:8080/#/c/12037/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12037/5//COMMIT_MSG@15 PS5, Line 15: simluated nit: typo http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/experiments/hash-ring-util.cc File be/src/experiments/hash-ring-util.cc: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/experiments/hash-ring-util.cc@39 PS5, Line 39: hashspace nit: hashspace or hash space (line below) http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/experiments/hash-ring-util.cc@44 PS5, Line 44: HashRingUtil Can you think of a more descriptive name? HashRingConsistencyCheck or similar? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/backend-config.h File be/src/scheduling/backend-config.h: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/backend-config.h@33 PS5, Line 33: /// hostnames to IP addresses. Maybe mention the hash ring in the class comment, too? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/backend-config.h@83 PS5, Line 83: HashRing backend_ip_hashring_; nit: hash_ring? Or Hashring? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/backend-config.cc File be/src/scheduling/backend-config.cc: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/backend-config.cc@23 PS5, Line 23: static const uint32_t NUM_HASHRING_REPLICAS = 25; How did you pick 25? Does this number have to have any special properties, e.g. not be a power of 2? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring-test.cc File be/src/scheduling/hash-ring-test.cc: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring-test.cc@42 PS5, Line 42: void VerifyTotalAllocation(const vector& addresses, I think these methods could use a comment, some of them weren't clear to me from their names alone. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h File be/src/scheduling/hash-ring.h: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@31 PS5, Line 31: TNetworkAddress For the purpose of scheduling we use IP addresses because whether a read is local or remote does not depend on the port. Should we use the same type here, too? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@45 PS5, Line 45: /// node is remapped. This allows for bounded disruption. Should we quantify the bound? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@64 PS5, Line 64: for (auto it = hash_ring.node_hashmap_.begin(); nit: you could use a range-based for loop, here and elsewhere. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@76 PS5, Line 76: HashRing(HashRing&&) = delete; Deleting a ctor seems uncommon enough to warrant a comment. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@97 PS5, Line 97: /// this TNetworkAddress. This is useful for examining how the hash space is balanced Can you include in the comment how the portion is represented in the int64_t, i.e. that it is sum of all range sizes? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@99 PS5, Line 99: void GetDistributionMap(std::map& distribution_map) const; Pass output parameter by pointer. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@106 PS5, Line 106: std::set nodes_; Should we do this in the backend config, possibly in a vector, and then just store indexes in this class? The scheduler's performance also suffers from the prolific use of strings and network addresses in various maps and could benefit from such a mapping in BackendConfig. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@111 PS5, Line 111: /// of the two underlying TNetworkAddress's. Collisions should be rare, so this will This looks like the birthday paradox and collisions might be more common that intuition suggests. Hashing 372 nodes 25 times each to 32bit would have a >1% probability of a collision. Please also see my comment in the implementation of AddNode on whether a collision of a node means that the subsequent hashes would also collide. http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@113 PS5, Line 113: std::map node_hashmap_; It seems a bit confusing that a ordered map is called hash_map. Can you think of a different name, e.g. hash_to_node_? http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.cc File be/src/scheduling/hash-ring.cc: http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.cc@57 PS5, Line 57: node_hashmap_[hash_val] = node_it; If there's a collision, then another node
[Impala-ASF-CR] IMPALA-8148: Misc. FE code cleanup
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12317 ) Change subject: IMPALA-8148: Misc. FE code cleanup .. Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/12317/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12317/3//COMMIT_MSG@13 PS3, Line 13: Tests: Reran all FE tests to verify no regressions. Change-Id should be at the bottom. http://gerrit.cloudera.org:8080/#/c/12317/3/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/12317/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@101 PS3, Line 101: com.google.common.base.Predicate Probably not in this patch, should we consider switching to java.util.function.Predicate instead? -- To view, visit http://gerrit.cloudera.org:8080/12317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e2f2e79f36e804fd592b6fe77f0554c5ca803eb Gerrit-Change-Number: 12317 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 19:48:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8148: Misc. FE code cleanup
Hello Bharath Vissapragada, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12317 to look at the new patch set (#3). Change subject: IMPALA-8148: Misc. FE code cleanup .. IMPALA-8148: Misc. FE code cleanup Roll-up of a bunch of minor code cleanup and refactoring. Pulled out of other patches to keep those patches focused on a single change. Change-Id: I4e2f2e79f36e804fd592b6fe77f0554c5ca803eb Tests: Reran all FE tests to verify no regressions. --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/common/InvalidValueException.java M fe/src/main/java/org/apache/impala/common/TreeNode.java M fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M tests/common/skip.py M tests/util/test_file_parser.py 23 files changed, 136 insertions(+), 115 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/12317/3 -- To view, visit http://gerrit.cloudera.org:8080/12317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4e2f2e79f36e804fd592b6fe77f0554c5ca803eb Gerrit-Change-Number: 12317 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12296 )
Change subject: IMPALA-8093: Prefix time series counters with a hyphen
..
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@13
PS6, Line 13: Add TimeSeriesCounter prefix checks in test_observability tests.
nit: trailing whitespace
http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@19
PS6, Line 19: …
I'm not sure whether these ... in the commit message might cause issues for
some tools since they're not ascii characters. To be on the safe side you could
replace them with three single dots.
http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:
http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py@404
PS6, Line 404: assert re.search(" MemoryUsage", profile) is None
I don't think you need a regex here but you could just do "assert pattern in
profile" like below
--
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Comment-Date: Fri, 01 Feb 2019 19:27:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12334 Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure .. IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure I was able to reproduce this issue by running AuditingTest individually. Running all tests did not seem to reproduce this issue, which may be due to the test depends on the state from other tests. Looking at the code, the test was poorly written mainly because of two reasons: - The Sentry config was set to an empty string, which is not a valid config file. - Calling AuthorizationConfig.validateConfig() was missing. I also fixed a bug in AuthorizationConfig.validateConfig(), which skipped the Sentry config validation when the Sentry config file path is empty. As a result, I updated other tests that considered empty Sentry config file path to be valid. Testing: - Ran AuditingTest individually - Ran all FE tests Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 3 files changed, 25 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/12334/2 -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] Revert "IMPALA-8146: Remove make {debug,release,asan}.sh"
Michael Ho has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/12335 )
Change subject: Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
..
Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
This reverts commit e8ea4b0525fb3223da9291775eff4b7fbd15c547.
Apparently some users of Impala have some workflow which rely
on these scripts. Reverting this change to unblock those users.
Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Reviewed-on: http://gerrit.cloudera.org:8080/12335
Reviewed-by: Philip Zeyliger
Tested-by: Michael Ho
---
A bin/make_asan.sh
A bin/make_debug.sh
A bin/make_release.sh
3 files changed, 60 insertions(+), 0 deletions(-)
Approvals:
Philip Zeyliger: Looks good to me, approved
Michael Ho: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Revert "IMPALA-8146: Remove make {debug,release,asan}.sh"
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/12335 )
Change subject: Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
..
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 01 Feb 2019 19:31:33 +
Gerrit-HasComments: No
[Impala-ASF-CR] Revert "IMPALA-8146: Remove make {debug,release,asan}.sh"
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/12335 )
Change subject: Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
..
Patch Set 1: Code-Review+2
I recommend self-verifying and pushing ahead.
--
To view, visit http://gerrit.cloudera.org:8080/12335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 01 Feb 2019 19:29:02 +
Gerrit-HasComments: No
[Impala-ASF-CR] Revert "IMPALA-8146: Remove make {debug,release,asan}.sh"
Michael Ho has uploaded this change for review. (
http://gerrit.cloudera.org:8080/12335
Change subject: Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
..
Revert "IMPALA-8146: Remove make_{debug,release,asan}.sh"
This reverts commit e8ea4b0525fb3223da9291775eff4b7fbd15c547.
Apparently some users of Impala have some workflow which rely
on these scripts. Reverting this change to unblock those users.
Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
---
A bin/make_asan.sh
A bin/make_debug.sh
A bin/make_release.sh
3 files changed, 60 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12335/1
--
To view, visit http://gerrit.cloudera.org:8080/12335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibcb921224071eef2dc77cf7b9f4c618f88aaa7c1
Gerrit-Change-Number: 12335
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
[Impala-ASF-CR] IMPALA-7985: Port RemoteShutdown() to KRPC.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12260 ) Change subject: IMPALA-7985: Port RemoteShutdown() to KRPC. .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1961/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4fd00ee4e638f5e71e27893162fd65501ef9e74e Gerrit-Change-Number: 12260 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 01 Feb 2019 19:10:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7980: Fix spinning because of buggy num unqueued files .
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12097 ) Change subject: IMPALA-7980: Fix spinning because of buggy num_unqueued_files_. .. Patch Set 9: Verified+1 Code-Review+2 Carrying +2. I've run tests on this manually, so I'm manually verifying. This needs a push to impala-lzo at the same time, so wouldn't pass the pre-commit anyway. -- To view, visit http://gerrit.cloudera.org:8080/12097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I133de13238d3d05c510e2ff771d48979125735b1 Gerrit-Change-Number: 12097 Gerrit-PatchSet: 9 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 01 Feb 2019 19:03:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7980: Fix spinning because of buggy num unqueued files .
Philip Zeyliger has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/12097 )
Change subject: IMPALA-7980: Fix spinning because of buggy num_unqueued_files_.
..
IMPALA-7980: Fix spinning because of buggy num_unqueued_files_.
This commit removes num_unqueued_files_ and replaces it with a more
tightly scoped and easier to reason about
remaining_scan_range_submissions_ variable. This variable (and its
predecessor) are used as a way to signal to scanner threads they may
exit (instead of spinning) because there will never be a scan range
provided to them, because no more scan ranges will be added. In
practice, most scanner implementations can never call AddDiskIoRanges()
after IssueInitialRanges(). The exception is sequence files and Avro,
which share a common base class. Instead of incrementing and
decrementing this counter in a variety of paths, this commit makes the
common case simple (set to 1 initially; decrement at exit points of
IssueInitialRanges()) and the complicated, sequence-file case is treated
within base-sequence-scanner.cc.
Note that this is not the first instance of a subtle bug
in this code. The following two JIRAs (and corresponding
commits) are fundamentally similar bugs:
IMPALA-3798: Disable per-split filtering for sequence-based scanners
IMPALA-1730: reduce scanner thread spinning windows
We ran into this bug when running TPC-DS query 1 on scale factor 10,000
(10TB) on a 140-node cluster with replica_preference=remote, we observed
really high system CPU usage for some of the scan nodes:
HDFS_SCAN_NODE (id=6):(Total: 59s107ms, non-child: 59s107ms, % non- child:
100.00%
- BytesRead: 80.50 MB (84408563)
- ScannerThreadsSysTime: 36m17s
Using 36 minutes of system time in only 1 minute of wall-clock time
required ~30 threads to be spinning in the kernel. We were able to use
perf to find a lot of usage of futex_wait() and pthread_cond_wait().
Eventually, we figured out that ScannerThreads, once started, loop
forever looking for work. The case that there is no work is supposed to
be rare, and the scanner threads are supposed to exit based on
num_unqueued_files_ being 0, but, in some cases, that counter isn't
appropriately decremented.
The reproduction is any query that uses runtime filters to filter out
entire files. Something like:
set RUNTIME_FILTER_WAIT_TIME_MS=1;
select count(*)
from customer
join customer_address on c_current_addr_sk = ca_address_sk
where ca_street_name="DoesNotExist" and c_last_name="DoesNotExist";
triggers this behavior. This code path is covered by several existing
tests, most directly in test_runtime_filters.py:test_file_filtering().
Interestingly, though this wastes cycles, query results are unaffected.
I initially fixed this bug with a point fix that handled the case when
runtime filters caused files to be skipped and added assertions that
checked that num_unqueued_files_ was decremented to zero when queries
finished. Doing this led me, somewhat slowly, to both finding similar
bugs in other parts of the code (HdfsTextScanner::IssueInitialRanges had
the same bug if the entire file was skipped) and fighting with races on
the assertion itself. I eventually concluded that there's really no
shared synchronization between progress_.Done() and num_unqueued_files_.
The same conclusion is true for the current implementation, so there
aren't assertions.
I added a metric for how many times the scanners run through their
loop without doing any work and observed it to be non-zero
for a query from tests/query_test/test_runtime_filters.py:test_wait_time.
To measure the effect of this, I set up a cluster of 9 impalad's and
1 coordinator, running against an entirely remote HDFS. The machines
were r4.4xlarge and the remote disks were EBS st1's, though everything
was likely buffer cached. I ran
TPCDS-Q1 with RUNTIME_FILTER_WAIT_TIME_MS=2000 against
tpcds_1000_decimal_parquet 10 times. The big observable
thing is that ScannerThreadsSysTime went from 5.6 seconds per
query to 1.9 seconds per query. (I ran the text profiles through the
old-fashioned:
grep ScannerThreadsSysTime profiles | awk '/ms/ { x += $3/1000 } /ns/ { x +=
$3/100 } END { print x }'
)
The query time effect was quite small (the fastest query was 3.373s
with the change and 3.82s without the change, but the averages were
tighter), but the extra work was visible in the profiles.
I happened to rename HdfsScanNode::file_type_counts_ to
HdfsScanNode::file_type_counts_lock_ because
HdfsScanNodeBase::file_type_counts_ also exists, and
is totally different.
This bug was co-debugged by Todd Lipcon, Joe McDonnell, and Philip
Zeyliger.
Change-Id: I133de13238d3d05c510e2ff771d48979125735b1
Reviewed-on: http://gerrit.cloudera.org:8080/12097
Reviewed-by: Philip Zeyliger
Tested-by: Philip Zeyliger
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M
[Impala-ASF-CR] IMPALA-8092: Add an admission controller debug page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12244 ) Change subject: IMPALA-8092: Add an admission controller debug page .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3698/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Gerrit-Change-Number: 12244 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 18:47:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8092: Add an admission controller debug page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12244 ) Change subject: IMPALA-8092: Add an admission controller debug page .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Gerrit-Change-Number: 12244 Gerrit-PatchSet: 7 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 18:47:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7985: Port RemoteShutdown() to KRPC.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12260 ) Change subject: IMPALA-7985: Port RemoteShutdown() to KRPC. .. Patch Set 5: (4 comments) Thanks Michael for comments. I think in general I would prefer to finish this change and do some followup work in IMPALA-8143. Or I can do that work as part of this change, what do you think? http://gerrit.cloudera.org:8080/#/c/12260/4/be/src/service/control-service.h File be/src/service/control-service.h: http://gerrit.cloudera.org:8080/#/c/12260/4/be/src/service/control-service.h@84 PS4, Line 84: ic Status DoRpcWithRet > Instead of passing the MonoDelta object itself, how about just defining thi My experience in the Java world with TimeUnit is that using a type like MonoDelta makes code more readable and avoids mistakes. http://gerrit.cloudera.org:8080/#/c/12260/4/be/src/service/control-service.h@85 PS4, Line 85: const char* debug_actio > DCHECK_GT() Done http://gerrit.cloudera.org:8080/#/c/12260/4/be/src/service/control-service.h@92 PS4, Line 92: // Check for injected failures. > Please add a TODO about sleeping between retries in case the server is over I added the TODO, thanks. I didn't add the actual sleep as that was what we agreed in our offline meeting, I though we said we would do this work in a followup jira, which I added: IMPALA-8143. I agree that the code is fairly simple... but there are unresolved questions: how long should the sleep be? Do we want exponential backoff? How can we test this? http://gerrit.cloudera.org:8080/#/c/12260/3/be/src/service/control-service.cc File be/src/service/control-service.cc: http://gerrit.cloudera.org:8080/#/c/12260/3/be/src/service/control-service.cc@170 PS3, Line 170: FAULT_INJECTION_RPC_DELAY(RPC_CANCELQUERYFINSTANCES); > This can easily be replaced by DebugAction. Please see https://github.com/a OK, how about I promise to do this in IMPALA-8143? I can see some other problems with test_rpc_timeout.py that need to be fixed and I will need to add some sort of test for the new timeout. -- To view, visit http://gerrit.cloudera.org:8080/12260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4fd00ee4e638f5e71e27893162fd65501ef9e74e Gerrit-Change-Number: 12260 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Fri, 01 Feb 2019 18:27:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7985: Port RemoteShutdown() to KRPC.
Andrew Sherman has uploaded a new patch set (#5). (
http://gerrit.cloudera.org:8080/12260 )
Change subject: IMPALA-7985: Port RemoteShutdown() to KRPC.
..
IMPALA-7985: Port RemoteShutdown() to KRPC.
The :shutdown command is used to shutdown a remote server. The common
case is that a user specifies the impalad to shutdown by specifying a
host e.g. :shutdown('host100'). If a user has more than one impalad on a
remote host then the form :shutdown(':') can be used to
specify the port by which the impalad can be contacted. Prior to
IMPALA-7985 this port was the backend port, e.g.
:shutdown('host100:22000'). With IMPALA-7985 the port to use is the KRPC
port, e.g. :shutdown('host100:27000').
Shutdown is implemented by making an rpc call to the target impalad.
This changes the implementation of this call to use KRPC.
To aid the user in finding the KRPC port, the KRPC address is added to
the /backends section of the debug web page.
We attempt to detect the case where :shutdown is pointed at a thrift
port (like the backend port) and print an informative message.
Documentation of this change will be done in IMPALA-8098.
Further improvements to DoRpcWithRetry() will be done in IMPALA-8143.
For discussion of why it was chosen to implement this change in an
incompatible way, see comments in
https://issues.apache.org/jira/browse/IMPALA-7985.
TESTING
Ran all end-to-end tests.
Add a test for /backends to test_web_pages.py.
In test_restart_services.py add a call to the old backend port to the
test. Some expected error messages were changed in line with what KRPC
returns.
Change-Id: I4fd00ee4e638f5e71e27893162fd65501ef9e74e
---
M be/src/runtime/backend-client.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M common/protobuf/control_service.proto
M common/thrift/ImpalaInternalService.thrift
M tests/custom_cluster/test_restart_services.py
M tests/webserver/test_web_pages.py
M www/backends.tmpl
16 files changed, 229 insertions(+), 144 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/12260/5
--
To view, visit http://gerrit.cloudera.org:8080/12260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4fd00ee4e638f5e71e27893162fd65501ef9e74e
Gerrit-Change-Number: 12260
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR](asf-site) IMPALA-7771: remove source link from Downloads
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/12333 ) Change subject: IMPALA-7771: remove source link from Downloads .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: I9ec071ae3affc3ae6aaaea15a1243ab665749cf3 Gerrit-Change-Number: 12333 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Fri, 01 Feb 2019 17:49:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/12296 )
Change subject: IMPALA-8093: Prefix time series counters with a hyphen
..
Patch Set 6:
(3 comments)
Patch set 6 should address all the issues.
http://gerrit.cloudera.org:8080/#/c/12296/4//COMMIT_MSG
Commit Message:
PS4:
> Please explain in the commit message why we make this change (consistency),
Done
http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:
http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc@777
PS4, Line 777: stream << prefix << " - " << v.first << "("
> The amount of indent here looks different from the SummaryStatsCounters bel
I used the same prefix as event timers above. Published new fix to make it the
same as SummaryStatsCounter.
http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:
http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py@429
PS1, Line 429: @classmethod
> This is non-deterministic because the query runs so fast that not a single
For this seq/snam/block format, this query can go into the if statement 100%
with my local test. I used the test to setup the format to slow down the join
to let the counter appear in the profile. And it is clearer to let more test
works for a jira.
--
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Comment-Date: Fri, 01 Feb 2019 17:10:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1960/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Fri, 01 Feb 2019 16:17:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5861: fix RowsRead for zero-slot table scan
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12332 ) Change subject: IMPALA-5861: fix RowsRead for zero-slot table scan .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1959/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12332 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7a927c6a4f0b8055608cb7a5e2b550a1610cef89 Gerrit-Change-Number: 12332 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 01 Feb 2019 15:49:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12296 to look at the new patch set (#6). Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. IMPALA-8093: Prefix time series counters with a hyphen The change makes profiles prefix counters consistent. Only TimeSeriesCounters are affected. Testing: Add TimeSeriesCounter prefix checks in test_observability tests. Manual Tests: Run query and check profile for MemoryUsage and ThreadUsage. Following section of profile shows that TimeSeriesCounters are consistent with other counters: Fragment F00: … Fragment Instance Lifecycle Event Timeline: 273.841ms - Prepare Finished: 1.511ms (1.511ms) … - MemoryUsage(500.000ms): 2.81 MB - ThreadUsage(500.000ms): 1 - AverageThreadTokens: 1.00 - BloomFilterBytes: 1.00 MB (1048576) - ExchangeScanRatio: 0.00 - PeakMemoryUsage: 54.26 MB (56891933) - PeakReservation: 53.00 MB (55574528) Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa --- M be/src/util/runtime-profile.cc M tests/query_test/test_observability.py 2 files changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/6 -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR](asf-site) IMPALA-7771: remove source link from Downloads
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12333 Change subject: IMPALA-7771: remove source link from Downloads .. IMPALA-7771: remove source link from Downloads This is arguably a violation of ASF policy (see JIRA). In any case it isn't important when the source is already linked from the navbar. Change-Id: I9ec071ae3affc3ae6aaaea15a1243ab665749cf3 --- M downloads.html 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/12333/1 -- To view, visit http://gerrit.cloudera.org:8080/12333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: newchange Gerrit-Change-Id: I9ec071ae3affc3ae6aaaea15a1243ab665749cf3 Gerrit-Change-Number: 12333 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7999: clean up start-*d.sh scripts
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12271 ) Change subject: IMPALA-7999: clean up start-*d.sh scripts .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1958/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib67444fd4def8da119db5d3a0832ef1de15b068b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 15:29:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1957/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Fri, 01 Feb 2019 15:29:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5861: fix RowsRead for zero-slot table scan
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12332 Change subject: IMPALA-5861: fix RowsRead for zero-slot table scan .. IMPALA-5861: fix RowsRead for zero-slot table scan Testing: Added regression test based on JIRA. Change-Id: I7a927c6a4f0b8055608cb7a5e2b550a1610cef89 --- M be/src/exec/parquet/hdfs-parquet-scanner.cc M testdata/workloads/functional-query/queries/QueryTest/mixed-format.test 2 files changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/12332/1 -- To view, visit http://gerrit.cloudera.org:8080/12332 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7a927c6a4f0b8055608cb7a5e2b550a1610cef89 Gerrit-Change-Number: 12332 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-4018 Part1: Add FORMAT clause in CAST()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12267 ) Change subject: IMPALA-4018 Part1: Add FORMAT clause in CAST() .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1956/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia514aaa9e8f5487d396587d5ed24c7348a492697 Gerrit-Change-Number: 12267 Gerrit-PatchSet: 4 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 01 Feb 2019 15:07:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4018 Part1: Add FORMAT clause in CAST()
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/12267 )
Change subject: IMPALA-4018 Part1: Add FORMAT clause in CAST()
..
Patch Set 4:
(11 comments)
Thanks everyone for taking a look and leaving comments!
Paul, those are excellent questions about compatibility! Let me move that
conversation back to the Jira so that we get Greg's opinion as well.
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/CMakeLists.txt
File be/src/exprs/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/CMakeLists.txt@33
PS3, Line 33: cast-functi
> Is adding header files necessary?
Done
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/cast-functions-ir.cc@167
PS3, Line 167: const
> const string
Done
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/cast-functions-ir.cc@172
PS3, Line 172: DateTimeFormatContext format_ctx(cast_format.c_str(),
cast_format.length());
: if (!ParseFormatTokens(_ctx)) return
StringVal::null();
> I am concerned about the performance impact of doing this for every row - T
Excellent point!
I'll dig a bit into this. I'm sure the parsing can be done before we reach the
cast functions. What I don't know from the top of my head is that how can I
return an error or null in case of a parse error. (StringVal::null() vs
TimestampVal::null())
I'll be back with the outcome, thx!
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/cast-functions-ir.cc@283
PS3, Line 283: const
> const string
Done
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/scalar-expr-evaluator.cc
File be/src/exprs/scalar-expr-evaluator.cc:
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/scalar-expr-evaluator.cc@129
PS3, Line 129: string cast_format = root_.GetCastFormat();
> I am not too familiar with ScalarExprEvaluator, but this functions seems st
That's again a good catch. The above scenarios are broken now. Let me sort this
out and get back.
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/scalar-expr.h
File be/src/exprs/scalar-expr.h:
http://gerrit.cloudera.org:8080/#/c/12267/3/be/src/exprs/scalar-expr.h@162
PS3, Line 162: std::s
> In headers use fully qualified names: std::string
Done
http://gerrit.cloudera.org:8080/#/c/12267/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/12267/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@220
PS2, Line 220: return Objects.toStringHelper(this)
> Probably doable. As you noted, we load functions on startup. Cast functions
Yeah, I think this is kind of what I wrote above :) But it won't be enough to
change CastExpr.initBuiltins() to make this work because the Expr tree has to
also be extended with an additional child node. This is needed because the
parameters of the cast functions on the BE are gathered from there. And then we
end up with adding "if this is a string vs timestamp cast than do this
otherwise do that" multiple locations of the code that I don't find future
proof.
In addition Csaba had a good point: It's not the best idea to do the format
parsing in the cast functions because that would be run for each row. So if we
want to make this more efficient than we can't pass the format as it is to the
cast functions as parsing should happen before we reach them.
http://gerrit.cloudera.org:8080/#/c/12267/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/12267/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@88
PS3, Line 88: this(targetTypeDef, e, null);
: }
:
: public CastExpr(TypeDef targetTypeDef, Expr e, String format) {
: Preconditions.checkNotNull(targetTypeDef);
: Preconditions.che
> Can you call here 'this(targetTypeDef, e, null)' instead?
Done
http://gerrit.cloudera.org:8080/#/c/12267/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@228
PS3, Line 228: public boolean isImplicit() { return isImplicit_; }
:
> I think these two lines whould be switched
Done
http://gerrit.cloudera.org:8080/#/c/12267/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/12267/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2926
PS3, Line 2926: public void TestCastFormatClauseFromString() throws
AnalysisException {
: AnalysisError("select cast('05-01-2017' AS DATE FORMAT
'MM-dd-')",
: "Unsupported data type: DATE");
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12296 to look at the new patch set (#5). Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. IMPALA-8093: Prefix time series counters with a hyphen Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa --- M be/src/util/runtime-profile.cc M tests/query_test/test_observability.py 2 files changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/5 -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-7999: clean up start-*d.sh scripts
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12271 to look at the new patch set (#7). Change subject: IMPALA-7999: clean up start-*d.sh scripts .. IMPALA-7999: clean up start-*d.sh scripts Delete these wrapper scripts and replace with a generic start-daemon.sh script that sets environment variables without the other logic. Move the logic for setting JAVA_TOOL_OPTIONS into start-impala-cluster.py. Remove some options like -jvm_suspend, -gdb, -perf that may not be used. These can be reintroduced if needed. Port across the kerberized minicluster logic (which has probably bitrotted) in case it needs to be revived. Remove --verbose option that didn't appear to be useful (it claims to print daemon output to the console, but output is still redirected regardless). Removed a level of quoting in custom cluster test argument handling - this was made unnecessary by properly escaping arguments with pipes.escape() in run_daemon(). Testing: * Ran exhaustive tests. * TODO: run on CentOS 6 to confirm we didn't reintroduce Popen issue worked around by kwho. Change-Id: Ib67444fd4def8da119db5d3a0832ef1de15b068b --- D bin/start-catalogd.sh A bin/start-daemon.sh M bin/start-impala-cluster.py D bin/start-impalad.sh D bin/start-statestored.sh M tests/common/custom_cluster_test_suite.py M tests/common/impala_cluster.py M tests/custom_cluster/test_breakpad.py M tests/custom_cluster/test_redaction.py M tests/custom_cluster/test_scratch_disk.py 10 files changed, 157 insertions(+), 339 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/12271/7 -- To view, visit http://gerrit.cloudera.org:8080/12271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib67444fd4def8da119db5d3a0832ef1de15b068b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 ) Change subject: IMPALA-8093: Prefix time series counters with a hyphen .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12296/5/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/12296/5/tests/query_test/test_observability.py@403 PS5, Line 403: e flake8: E501 line too long (93 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/12296 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa Gerrit-Change-Number: 12296 Gerrit-PatchSet: 5 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Fri, 01 Feb 2019 14:44:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4018 Part1: Add FORMAT clause in CAST()
Hello Greg Rahn, Paul Rogers, Attila Jeges, Csaba Ringhofer, Impala Public
Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12267
to look at the new patch set (#4).
Change subject: IMPALA-4018 Part1: Add FORMAT clause in CAST()
..
IMPALA-4018 Part1: Add FORMAT clause in CAST()
This enhancement introduces FORMAT clause for CAST()
operator. The FORMAT clause is applicable for casts between
string types and timestamp types. The same format pattern is
accepted as what e.g. the to_timestamp() function accepts.
Usage example:
SELECT CAST('01-02-2019' AS TIMESTAMP FORMAT 'MM-dd-');
Change-Id: Ia514aaa9e8f5487d396587d5ed24c7348a492697
---
A be/src/exprs/cast-expr.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/scalar-expr-evaluator.cc
M be/src/exprs/scalar-expr-evaluator.h
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/udf/udf-internal.h
M be/src/udf/udf.cc
M be/src/udf/udf.h
M common/thrift/Exprs.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A testdata/workloads/functional-query/queries/QueryTest/cast_format.test
A tests/query_test/test_cast_with_format.py
21 files changed, 350 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/12267/4
--
To view, visit http://gerrit.cloudera.org:8080/12267
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia514aaa9e8f5487d396587d5ed24c7348a492697
Gerrit-Change-Number: 12267
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-8092: Add an admission controller debug page
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12244 ) Change subject: IMPALA-8092: Add an admission controller debug page .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff055d9709ea1bcc2f492adcde92241b6149f766 Gerrit-Change-Number: 12244 Gerrit-PatchSet: 6 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 14:14:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8137: [DOCS] Order By does not happens on one node
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12330 ) Change subject: IMPALA-8137: [DOCS] Order By does not happens on one node .. IMPALA-8137: [DOCS] Order By does not happens on one node Change-Id: If8d7bf26fffaf93982e67f8bc8f37742c81fda39 Reviewed-on: http://gerrit.cloudera.org:8080/12330 Tested-by: Impala Public Jenkins Reviewed-by: Tim Armstrong --- M docs/topics/impala_order_by.xml 1 file changed, 15 insertions(+), 14 deletions(-) Approvals: Impala Public Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If8d7bf26fffaf93982e67f8bc8f37742c81fda39 Gerrit-Change-Number: 12330 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8137: [DOCS] Order By does not happens on one node
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12330 ) Change subject: IMPALA-8137: [DOCS] Order By does not happens on one node .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12330 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8d7bf26fffaf93982e67f8bc8f37742c81fda39 Gerrit-Change-Number: 12330 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 14:15:28 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 13:59:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 16: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1955/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 16 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 12:08:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 15: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1954/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 15 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 11:51:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Hello Michael Ho, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12069 to look at the new patch set (#16). Change subject: IMPALA-7694: Add host resource usage metrics to profile .. IMPALA-7694: Add host resource usage metrics to profile This change adds a mechanism to collect host resource usage metrics to profiles. Metric collection can be controlled through a new query option 'RESOURCE_TRACE_RATIO'. It specifies the probability with which metrics collection will be enabled. Collection always happens per query for all executors that run one or more fragment instances of the query. This mechanism adds a new time series counter class that collects all measured values and does not re-sample them. It will re-sample values when printing them into a string profile, preserving up to 64 values, but Thrift profiles will contain the full list of values. We add a new section "Per Node Profiles" to the profile to store and show these values: Per Node Profiles: lv-desktop:22000: CpuIoWaitPercentage (500.000ms): 0, 0 CpuSysPercentage (500.000ms): 1, 1 CpuUserPercentage (500.000ms): 4, 0 - ScratchBytesRead: 0 - ScratchBytesWritten: 0 - ScratchFileUsedBytes: 0 - ScratchReads: 0 (0) - ScratchWrites: 0 (0) - TotalEncryptionTime: 0.000ns - TotalReadBlockTime: 0.000ns This change also uses the aforementioned mechanism to collect CPU usage metrics (user, system, and IO wait time). A future change can then add a tool to decode a Thrift profile and plot the contained usage metrics, e.g. using matplotlib (IMPALA-8123). Such a tool is not included in this change because it will require some reworking of the python dependencies. This change also includes a few minor improvements to make the resulting code more readable: - Extend the PeriodicCounterUpdater to call functions to update global metrics before updating the counters. - Expose the scratch profile within the per node resource usage section. - Improve documentation of the profile counter classes. - Remove synchronization from StreamingSampler. - Remove a few pieces of dead code that otherwise would have required updates. - Factor some code for profile decoding into the Impala python library Testing: This change contains a unit test for the system level metrics collection and e2e tests for the profile changes. Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 --- M be/src/kudu/util/logging.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/CMakeLists.txt M be/src/util/periodic-counter-updater.cc M be/src/util/periodic-counter-updater.h M be/src/util/pretty-printer.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M be/src/util/streaming-sampler.h A be/src/util/system-state-info-test.cc A be/src/util/system-state-info.cc A be/src/util/system-state-info.h M bin/parse-thrift-profile.py M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/Metrics.thrift M common/thrift/RuntimeProfile.thrift A lib/python/impala_py_lib/profiles.py M tests/beeswax/impala_beeswax.py M tests/common/impala_test_suite.py M tests/query_test/test_observability.py 37 files changed, 1,158 insertions(+), 243 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/12069/16 -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 16 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 15: PS16 rebases the change. Michael, Tim, do you want to have another look? Otherwise I'll take Phil's +2 and go with it. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 15 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 11:24:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/12069/11/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/12069/11/be/src/util/runtime-profile.h@419 PS11, Line 419: Samples : /// are not re-sampled into larger intervals, instead owners of this profile can call : /// ClearChunkedTimeSeriesCounters() to reset the sample buffers of all chunked time : /// series counters > Yes, capping it makes sense but warning about it "once" when it becomes ful I chose "every 60 seconds" instead of "once" to make sure that this gets logged again if the system experiences slowness after a while. "Once" would only log this one time during the lifetime of the daemon. Let me know if you prefer a different kind of frequency. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 15 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 01 Feb 2019 11:07:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Hello Michael Ho, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12069 to look at the new patch set (#15). Change subject: IMPALA-7694: Add host resource usage metrics to profile .. IMPALA-7694: Add host resource usage metrics to profile This change adds a mechanism to collect host resource usage metrics to profiles. Metric collection can be controlled through a new query option 'RESOURCE_TRACE_RATIO'. It specifies the probability with which metrics collection will be enabled. Collection always happens per query for all executors that run one or more fragment instances of the query. This mechanism adds a new time series counter class that collects all measured values and does not re-sample them. It will re-sample values when printing them into a string profile, preserving up to 64 values, but Thrift profiles will contain the full list of values. We add a new section "Per Node Profiles" to the profile to store and show these values: Per Node Profiles: lv-desktop:22000: CpuIoWaitPercentage (500.000ms): 0, 0 CpuSysPercentage (500.000ms): 1, 1 CpuUserPercentage (500.000ms): 4, 0 - ScratchBytesRead: 0 - ScratchBytesWritten: 0 - ScratchFileUsedBytes: 0 - ScratchReads: 0 (0) - ScratchWrites: 0 (0) - TotalEncryptionTime: 0.000ns - TotalReadBlockTime: 0.000ns This change also uses the aforementioned mechanism to collect CPU usage metrics (user, system, and IO wait time). A future change can then add a tool to decode a Thrift profile and plot the contained usage metrics, e.g. using matplotlib (IMPALA-8123). Such a tool is not included in this change because it will require some reworking of the python dependencies. This change also includes a few minor improvements to make the resulting code more readable: - Extend the PeriodicCounterUpdater to call functions to update global metrics before updating the counters. - Expose the scratch profile within the per node resource usage section. - Improve documentation of the profile counter classes. - Remove synchronization from StreamingSampler. - Remove a few pieces of dead code that otherwise would have required updates. - Factor some code for profile decoding into the Impala python library Testing: This change contains a unit test for the system level metrics collection and e2e tests for the profile changes. Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 --- M be/src/kudu/util/logging.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/CMakeLists.txt M be/src/util/periodic-counter-updater.cc M be/src/util/periodic-counter-updater.h M be/src/util/pretty-printer.h M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile-test.cc M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M be/src/util/streaming-sampler.h A be/src/util/system-state-info-test.cc A be/src/util/system-state-info.cc A be/src/util/system-state-info.h M bin/parse-thrift-profile.py M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/Metrics.thrift M common/thrift/RuntimeProfile.thrift A lib/python/impala_py_lib/profiles.py M tests/beeswax/impala_beeswax.py M tests/common/impala_test_suite.py M tests/query_test/test_observability.py 37 files changed, 1,158 insertions(+), 244 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/12069/15 -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 15 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12296 )
Change subject: IMPALA-8093: Prefix time series counters with a hyphen
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12296/4//COMMIT_MSG
Commit Message:
PS4:
Please explain in the commit message why we make this change (consistency),
that only time series counters are affected, and how you tested it. It would
also be good to put a small subsection of a profile into the commit message
that shows that after this change all counters are consistent.
http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:
http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc@777
PS4, Line 777: stream << prefix << " - " << v.first << "("
The amount of indent here looks different from the SummaryStatsCounters below,
can you please double check that it's correct here? It also seems different
from the plain counters in L1155.
--
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Fri, 01 Feb 2019 10:35:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3697/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 10:01:45 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1953/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 09:57:54 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12331 ) Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. Patch Set 1: This is a merge of picks from IMPALA-7143 and IMPALA-7144. IMPALA-7143 just comments out some flaky tests. IMPALA-7144 fixed the flakiness of these tests. -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 09:20:32 + Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7144: Re-enable TestDescribeTableResults
Hello Impala Public Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12331 to review the following change. Change subject: IMPALA-7144: Re-enable TestDescribeTableResults .. IMPALA-7144: Re-enable TestDescribeTableResults This patch makes the TestDescribeTableResults more robust by only comparing the information that the authorization cares about instead of comparing all output in DESCRIBE. This change will avoid any unnecessary changes to AuthorizationTest if HMS updates the DESCRIBE output. The test is also updated to support standalone execution without relying on other tests be executed first since it can cause the test to be flaky especially if the tests in AuthorizationTest are executed in parallel. Testing: - Ran all FE tests Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Reviewed-on: http://gerrit.cloudera.org:8080/10643 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 1 file changed, 174 insertions(+), 170 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/12331/1 -- To view, visit http://gerrit.cloudera.org:8080/12331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: newchange Gerrit-Change-Id: I3aeaecf5b6d906a66d338e165a6d506e3964563f Gerrit-Change-Number: 12331 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR](2.x) IMPALA-6479: Update DESCRIBE to respect column privileges
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12292 ) Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges .. IMPALA-6479: Update DESCRIBE to respect column privileges Modified the Frontend to filter columns from the DESCRIBE statement. Additionally, if a user has select on at least one column, they can run DESCRIBE and see most metadata. If they do not have full table access, they will not see location or view query metadata. Testing: Added tests to validate users that have one or more column access can run describe and that the output is filtered accordingly. Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 Reviewed-on: http://gerrit.cloudera.org:8080/9276 Reviewed-by: Alex Behm Tested-by: Impala Public Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/12292 Reviewed-by: Fredy Wijaya Tested-by: Impala Public Jenkins --- M be/src/service/client-request-state.cc M be/src/service/frontend.cc M be/src/service/frontend.h M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/catalog/Column.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java 13 files changed, 331 insertions(+), 69 deletions(-) Approvals: Fredy Wijaya: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: merged Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 Gerrit-Change-Number: 12292 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR](2.x) IMPALA-6479: Update DESCRIBE to respect column privileges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12292 ) Change subject: IMPALA-6479: Update DESCRIBE to respect column privileges .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: 2.x Gerrit-MessageType: comment Gerrit-Change-Id: Ic96ae184fccdc88ba970b5adcd501da1966accb9 Gerrit-Change-Number: 12292 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 01 Feb 2019 08:26:11 + Gerrit-HasComments: No
