[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5360/ -- To view, visit http://gerrit.cloudera.org:8080/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 07:02:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Impala Public Jenkins has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
..
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework makes a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happen.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of CreatePolicy requests, so increases the possibility of
CreatePolicy requests running concurrently with GetPolicies requests
polling from other ranger plugin instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Reviewed-on: http://gerrit.cloudera.org:8080/15235
Reviewed-by: Csaba Ringhofer
Tested-by: Impala Public Jenkins
---
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
Approvals:
Csaba Ringhofer: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 ) Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06 Gerrit-Change-Number: 15235 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 19 Feb 2020 06:38:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Gerrit-Change-Number: 15202 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 05:57:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. IMPALA-9373: Trial run of include-what-you-use Implemented recommendations from IWYU in a subset of files, mostly in util. Did a few cleanups related to systematic problems that I noticed as a result. I noticed that uid-util.h was pulling in boost UUID headers to a lot of compilation units, so refactored that a little bit, including pulling out the hash functions into unique-id-hash.h and moving some inline functions into client-request-state-map.cc. Systematically replaced the general boost mutex header with the internal pthread-based one. This is equivalent for us, since we assume that boost::mutex is implemented by pthread_mutex_t, e.g. for the implementation of ConditionVariable. Switch include guards to pragma once just as general cleanup. Prefix string with std:: consistently in headers so that they don't depend on "using" declarations pulled in from random headers. Look at includes of C++ stream headers, including iostream and stringstream, and replaced them with iosfwd or removed them if possible. Compile time: Measured a full ASAN build of the impalad binary on an 8 core machine with cccache enabled, but cleared. It used very slightly less CPU, probably because we are still pulling in most of the same system headers. Before: real9m27.502s user64m39.775s sys 2m49.002s After: real9m26.561s user64m28.948s sys 2m48.252s So for the moment, the only significant wins are on incremental builds, where touching header files should not require as many recompilations. Compile times should start to drop meaningfully once we thin out more unnecessary includes - currently it seems like most compile units end up with large chunks of boost/std code included via transitive header dependencies. Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Reviewed-on: http://gerrit.cloudera.org:8080/15202 Reviewed-by: Joe McDonnell Tested-by: Impala Public Jenkins --- M be/src/benchmarks/lock-benchmark.cc M be/src/benchmarks/process-wide-locks-benchmark.cc M be/src/catalog/catalog-server.h M be/src/codegen/llvm-codegen.cc M be/src/common/logging.cc M be/src/common/names.h M be/src/common/object-pool.h M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/data-sink.h M be/src/exec/exec-node.cc M be/src/exec/exec-node.h M be/src/exec/hash-table.h M be/src/exec/hbase-table-scanner.h M be/src/exec/hdfs-plugin-text-scanner.h M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scanner.h M be/src/exec/parquet/parquet-column-chunk-reader.h M be/src/exec/parquet/parquet-column-stats.inline.h M be/src/exec/parquet/parquet-level-decoder.h M be/src/exec/parquet/parquet-metadata-utils.h M be/src/exec/parquet/parquet-page-reader.h M be/src/exec/plan-root-sink.cc M be/src/exec/read-write-util.h M be/src/exec/scanner-context.h M be/src/exec/write-stream.h M be/src/exec/write-stream.inline.h M be/src/experiments/data-provider.h M be/src/exprs/operators-ir.cc M be/src/rpc/auth-provider.h M be/src/rpc/impala-service-pool.cc M be/src/rpc/thrift-client.h M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/runtime/bufferpool/buffer-pool-internal.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/client-cache.h M be/src/runtime/collection-value-builder.h M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/date-parse-util.h M be/src/runtime/decimal-test.cc M be/src/runtime/decimal-value.inline.h M be/src/runtime/descriptors.h M be/src/runtime/dml-exec-state.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/hdfs-fs-cache.h M be/src/runtime/initial-reservations.cc M be/src/runtime/io/disk-io-mgr-stress.cc M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/io/handle-cache.h M be/src/runtime/io/local-file-system-with-fault-injection.h M be/src/runtime/io/request-ranges.h M be/src/runtime/krpc-data-stream-mgr.h M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.h M be/src/runtime/krpc-data-stream-sender.h M be/src/runtime/lib-cache.h M be/src/runtime/mem-tracker.h M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-exec-mgr.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/raw-value.h M be/src/runtime/row-batch.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter.h M be/src/runtime/scanner-mem-limiter.h M be/src/runtime/sorted-run-merger.h M be/src/runtime/string-search.h M be/src/runtime/thread-resource-mgr.h M be/src/runtime/tmp-file-mgr.h M be/src/scheduling/executor-group.h M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.h M be/src/service/CMakeLists.txt M
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5266/ : 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/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 6 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 19 Feb 2020 03:09:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5265/ : 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/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 03:07:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4224: execute separate join builds fragments
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14859 ) Change subject: IMPALA-4224: execute separate join builds fragments .. Patch Set 42: Code-Review+2 (8 comments) http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/blocking-join-node.h File be/src/exec/blocking-join-node.h: http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/blocking-join-node.h@159 PS42, Line 159: /// Called by BlockingJoinNode after opening child(1) succeeds and before : /// SendBuildInputToSink is called to allocate resources for this ExecNode. update comment to accommodate separate builder case. http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.h@292 PS42, Line 292: DoneProbingHashPartitions nit: might be worth mentioning here and in the method below, that the probe_client returns the memory to the builder that it initially borrowed http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.h@570 PS42, Line 570: in nit: to state? http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.cc@607 PS42, Line 607: // An extra buffer is needed for reading spilled input stream, unless we're doing the : // initial partitioning of rows from the left child. nit: no need for this, instead add a comment in CalcProbeStreamReservation instead. See my next comment. http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.cc@623 PS42, Line 623: need_probe_buffer; nit: add comment: "We need a read buffer if the input is a spilled partition (i.e. that we are repartitioning the input)" http://gerrit.cloudera.org:8080/#/c/14859/42/be/src/exec/partitioned-hash-join-builder.cc@955 PS42, Line 955: Status status = : probe_client->TransferReservationTo(buffer_pool_client_, bytes, ); should we dcheck here whether we got all the memory that was lent to the probe? http://gerrit.cloudera.org:8080/#/c/14859/33/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/14859/33/be/src/runtime/coordinator.cc@363 PS33, Line 363: src_node_id, > I added an explanatory comment. It is a little weird, but it didn't seem wo makes sense. http://gerrit.cloudera.org:8080/#/c/14859/33/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: http://gerrit.cloudera.org:8080/#/c/14859/33/common/thrift/PlanNodes.thrift@367 PS33, Line 367: > We're pretty inconsistent about this. I think it mostly doesn't matter. sounds good -- To view, visit http://gerrit.cloudera.org:8080/14859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4403c8e62d9c13854e7830602ee613f8efc80c58 Gerrit-Change-Number: 14859 Gerrit-PatchSet: 42 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 19 Feb 2020 03:06:04 + Gerrit-HasComments: Yes
[native-toolchain-CR] Make Kudu working on aarch64
huangtianhua...@gmail.com has abandoned this change. ( http://gerrit.cloudera.org:8080/14948 ) Change subject: Make Kudu working on aarch64 .. Abandoned wait for the Kudu patch to get into Kudu -- To view, visit http://gerrit.cloudera.org:8080/14948 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I273a839386de3e8cf6069c0bb0d3910041757a92 Gerrit-Change-Number: 14948 Gerrit-PatchSet: 5 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9383: Fix hang in hs2-http server with large, chunks request
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15218 ) Change subject: IMPALA-9383: Fix hang in hs2-http server with large, chunks request .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Gerrit-Change-Number: 15218 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 02:43:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9383: Fix hang in hs2-http server with large, chunks request
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15218 ) Change subject: IMPALA-9383: Fix hang in hs2-http server with large, chunks request .. IMPALA-9383: Fix hang in hs2-http server with large, chunks request The issue is that in each call to THttpTransport::read(), it always starts by calling refill(), which tries to read more data off the socket, but for chunked requests each call to read() only processes a single chunk. So, if more than one chunk is read off the socket at a time (which happens with large requests due to an algorithm that increases the amount of data read off the socket with each read), you can end up with more chunks still needing to be processed but no more data to read off the socket, and the next call to THttpTransport::read() will hang when it calls refill(). The solution is to not always call refill() at the beginning of each call to THttpTransport::read(). This works because the functions that actually process the data, readLine() and readContent() will call refill() themselves anyways if they actually need more data. Testing: - Added a BE test that uses curl to send chunked requests to the hs2 http server. Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Reviewed-on: http://gerrit.cloudera.org:8080/15218 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/rpc/CMakeLists.txt R be/src/rpc/hs2-http-test.cc M be/src/transport/THttpTransport.cpp 3 files changed, 65 insertions(+), 9 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/15218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Gerrit-Change-Number: 15218 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8013: switch from boost to std locks
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15208 ) Change subject: IMPALA-8013: switch from boost to std locks .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5263/ : 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/15208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I81d37490d05049919270eb6406fb3d787f78f92f Gerrit-Change-Number: 15208 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 02:37:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 3): Factor out common code for cache implementations
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15179 ) Change subject: IMPALA-8690 (prep 3): Factor out common code for cache implementations .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5264/ : 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/15179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67294244a3e8a2812f1482fe786bf7f8e6ce054e Gerrit-Change-Number: 15179 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 19 Feb 2020 02:34:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15170 ) Change subject: IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5262/ : 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/15170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I917f8352c9276373dd2761af986bf3487855271c Gerrit-Change-Number: 15170 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 19 Feb 2020 02:32:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15169 ) Change subject: IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5261/ : 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/15169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76 Gerrit-Change-Number: 15169 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 02:29:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. Patch Set 6: Hi all, please review this revised patch set, where I additionally bumped up CDH_BUILD_NUMBER to 1893632. I am currently verifying whether or not this patch would fail some tests and will update you once I have some concrete results. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 6 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 19 Feb 2020 02:25:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Fang-Yu Rao has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre This patch bumps up the version of Guava libraries from 14.0.1 to 28.1-jre. Due to some changes in Guava's API's, we modify the call sites accordingly. Note that in order to instruct the Java classes under the directory of $IMPALA_HOME/common/yarn-extras to use the new Guava libraries, we also explicitly added a dependency in the corresponding pom.xml file. In addition, in this patch we bumped up CDH_BUILD_NUMBER to 1893632 so as to use the Sentry compiled with the newer Guava libraries, allowing Impala to perform authorization via Sentry. Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 --- M bin/impala-config.sh M common/yarn-extras/pom.xml M common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.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/IsNullPredicate.java M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Column.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/DataSource.java M fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/DataPartition.java M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java M fe/src/main/java/org/apache/impala/planner/ScanNode.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java M impala-parent/pom.xml 49 files changed, 105 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/15214/6 -- To view, visit http://gerrit.cloudera.org:8080/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 6 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5360/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 02:09:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 02:09:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/15237 )
Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15237/4/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:
http://gerrit.cloudera.org:8080/#/c/15237/4/bin/bootstrap_system.sh@297
PS4, Line 297: SET_LD_LIBRARY_PATH="export
LD_LIBRARY_PATH=/usr/lib/${ARCH_NAME}-linux-gnu/${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"
line too long (116 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/15237
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf
Gerrit-Change-Number: 15237
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Feb 2020 02:07:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
zhaoren...@hotmail.com has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 Add arm64 openjdk version for Ubuntu 18.04 in bootstrap_system.sh Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf --- M bin/bootstrap_system.sh 1 file changed, 10 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/15237/4 -- To view, visit http://gerrit.cloudera.org:8080/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 ) Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5359/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06 Gerrit-Change-Number: 15235 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 19 Feb 2020 02:02:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8013: switch from boost to std locks
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15208 ) Change subject: IMPALA-8013: switch from boost to std locks .. Patch Set 6: This was a follow-on to the original IWYU patch that I did earlier, it's pretty mechanical too. -- To view, visit http://gerrit.cloudera.org:8080/15208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I81d37490d05049919270eb6406fb3d787f78f92f Gerrit-Change-Number: 15208 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:49:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 3): Factor out common code for cache implementations
Joe McDonnell has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/15179 ) Change subject: IMPALA-8690 (prep 3): Factor out common code for cache implementations .. IMPALA-8690 (prep 3): Factor out common code for cache implementations The existing cache implementation handles LRU/FIFO eviction algorithms, but it also implements several components that are useful for other cache implementations. Specifically, there is a simple hash table (HandleTable) and a simple sharding implementation (ShardedCache). These can be reused across other eviction algorithms by making them generic. This pulls them out of be/src/util/cache/cache.cc and into a new be/src/util/cache/cache-internal.h file. To make the HandleTable generic, this introduces a HandleBase class that contains common code between the implementations (such as the key, the value, the hash, etc). The HandleTable works on this base class, and the RLHandle now derives from HandleBase. To support this, Allocate/Free need to treat the handle as an object (calling constructors/destructors) rather than a treating it like a chunk of memory (or simple struct). ShardedCache is made generic by having cache implementations derive from a base CacheShard class that defines the appropriate methods needed by the sharding class. This is purely an interface, and the base class defines no functions. The existing CacheShard is renamed to RLCachedShard and derives from this class. Change-Id: I67294244a3e8a2812f1482fe786bf7f8e6ce054e --- A be/src/util/cache/cache-internal.h M be/src/util/cache/cache.cc M bin/rat_exclude_files.txt 3 files changed, 407 insertions(+), 286 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/79/15179/6 -- To view, visit http://gerrit.cloudera.org:8080/15179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I67294244a3e8a2812f1482fe786bf7f8e6ce054e Gerrit-Change-Number: 15179 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8013: switch from boost to std locks
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15208 Change subject: IMPALA-8013: switch from boost to std locks .. IMPALA-8013: switch from boost to std locks The locks are mostly drop-in replacements that wrap the same pthread implementations. I went through all of the boost mutex/lock/thread-related includes and updated them so that the boost mutex headers are not pulled in. E.g. is an uber-header that pulls in *all* of the boost mutex and thread headers. std::shared_lock doesn't exist until c++17, so we don't replace this. Compile Time: I confirmed that preprocessed files got slightly shorter, which translated into a slight compile time improvement for a full build. Before: real9m27.502s user64m39.775s sys 2m49.002s After: real9m19.071s user63m13.950s sys 2m45.066s Change-Id: I81d37490d05049919270eb6406fb3d787f78f92f --- M be/src/benchmarks/free-lists-benchmark.cc M be/src/benchmarks/lock-benchmark.cc M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-server.h M be/src/codegen/codegen-symbol-emitter.cc M be/src/codegen/codegen-symbol-emitter.h M be/src/codegen/instruction-counter-test.cc M be/src/codegen/llvm-codegen.cc M be/src/common/atomic-test.cc M be/src/common/logging.cc M be/src/common/names.h M be/src/common/object-pool.h M be/src/exec/blocking-join-node.h M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/blocking-plan-root-sink.h M be/src/exec/buffered-plan-root-sink.h M be/src/exec/external-data-source-executor.cc M be/src/exec/hbase-table-writer.h M be/src/exec/hdfs-scan-node-base.h M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node.h M be/src/exec/partitioned-hash-join-node.h M be/src/exec/plan-root-sink.cc M be/src/rpc/auth-provider.h M be/src/rpc/impala-service-pool.cc M be/src/rpc/impala-service-pool.h M be/src/rpc/rpc-trace.h M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/rpc/thrift-util.cc M be/src/runtime/bufferpool/buffer-allocator.cc M be/src/runtime/bufferpool/buffer-allocator.h M be/src/runtime/bufferpool/buffer-pool-internal.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/free-list.h M be/src/runtime/bufferpool/reservation-tracker.cc M be/src/runtime/bufferpool/reservation-tracker.h M be/src/runtime/client-cache.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.h M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/hbase-table-factory.cc M be/src/runtime/hbase-table-factory.h M be/src/runtime/hbase-table.h M be/src/runtime/hdfs-fs-cache.cc M be/src/runtime/hdfs-fs-cache.h M be/src/runtime/initial-reservations.cc M be/src/runtime/io/disk-io-mgr-internal.h M be/src/runtime/io/disk-io-mgr-stress.cc M be/src/runtime/io/disk-io-mgr-stress.h M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/io/handle-cache.h M be/src/runtime/io/handle-cache.inline.h M be/src/runtime/io/request-context.cc M be/src/runtime/io/request-context.h M be/src/runtime/io/request-ranges.h M be/src/runtime/io/scan-range.cc M be/src/runtime/krpc-data-stream-mgr.cc M be/src/runtime/krpc-data-stream-mgr.h M be/src/runtime/krpc-data-stream-recvr.cc M be/src/runtime/krpc-data-stream-recvr.h M be/src/runtime/lib-cache.cc M be/src/runtime/lib-cache.h M be/src/runtime/mem-tracker.h M be/src/runtime/query-exec-mgr.h M be/src/runtime/query-state.cc M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.h M be/src/runtime/runtime-filter.inline.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/scanner-mem-limiter.cc M be/src/runtime/sorted-run-merger.h M be/src/runtime/sorter.cc M be/src/runtime/thread-resource-mgr.cc M be/src/runtime/thread-resource-mgr.h M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/cluster-membership-mgr.h M be/src/scheduling/scheduler.h M be/src/service/child-query.h M be/src/service/client-request-state.h M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.h M be/src/statestore/failure-detector.cc M be/src/statestore/failure-detector.h M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M be/src/testutil/impalad-query-executor.h M be/src/transport/TSaslServerTransport.cpp M be/src/transport/TSaslServerTransport.h M be/src/util/blocking-queue-test.cc M be/src/util/blocking-queue.h M
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. Patch Set 5: Yep, I think there are a couple of angles - one is to go through and systematically apply the changes, the other is to cherry-pick some higher-impact changes. One thing that I noticed in the output is that it had some good suggestions about other boost headers like the date_time ones - e.g. we don't need to pull in all the headers for timestamp-value.h -- To view, visit http://gerrit.cloudera.org:8080/15202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Gerrit-Change-Number: 15202 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:48:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 3): Factor out common code for cache implementations
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15179 ) Change subject: IMPALA-8690 (prep 3): Factor out common code for cache implementations .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/15179/5/be/src/util/cache/cache-internal.h File be/src/util/cache/cache-internal.h: http://gerrit.cloudera.org:8080/#/c/15179/5/be/src/util/cache/cache-internal.h@269 PS5, Line 269: HandleType* h = reinterpret_cast(pending_handle); : delete h; > I ran ASAN, and it is unhappy about the asymmetry between using a new to al Reworked this code to make things symmetric. I also pushed the Allocate/Free calls down to the shards to eliminate a template parameter on the ShardedCache. -- To view, visit http://gerrit.cloudera.org:8080/15179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67294244a3e8a2812f1482fe786bf7f8e6ce054e Gerrit-Change-Number: 15179 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 19 Feb 2020 01:48:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation
Joe McDonnell has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/15170 ) Change subject: IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation .. IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation In a previous change, the Kudu caching code from be/src/kudu/util was copied to be/src/util/cache. This gets that code workable for Impala and uses it for the DataCache. This involves the following changes: 1. Fix up includes to point to Impala's cache implementation 2. Remove NVM Cache support and code (not used by Impala) 3. Remove CacheMetrics code (not used by Impala) 4. Move the cache code to the impala namespace 5. Modify cache-bench and cache-test to use Impala's backend test infrastructure 6. Switch DataCache to use be/src/util/cache These are only boilerplate changes. The cache implementation has not meaningfully changed. Testing: - Ran core tests - The modified version of Kudu's cache-test passes - The modified version of Kudu's cache-bench runs successfully Change-Id: I917f8352c9276373dd2761af986bf3487855271c --- M be/CMakeLists.txt M be/src/runtime/io/data-cache.cc M be/src/runtime/io/data-cache.h M be/src/util/CMakeLists.txt M be/src/util/cache/CMakeLists.txt M be/src/util/cache/cache-bench.cc M be/src/util/cache/cache-test.cc M be/src/util/cache/cache.cc M be/src/util/cache/cache.h D be/src/util/cache/nvm_cache.cc D be/src/util/cache/nvm_cache.h M bin/rat_exclude_files.txt 12 files changed, 71 insertions(+), 952 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/15170/7 -- To view, visit http://gerrit.cloudera.org:8080/15170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I917f8352c9276373dd2761af986bf3487855271c Gerrit-Change-Number: 15170 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15170 ) Change subject: IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG@16 PS6, Line 16: 3. Remove CacheMetrics code (not used by Impala) > I assume we have some other way of tracking these metrics? Or are they not We currently do not use them for the data cache, and we maintain equivalent metrics inside the data cache. We may want metrics directly from the cache code in future, but it would be easy to reintroduce this code using Impala metrics infrastructure (and avoid bringing in the Kudu dependencies). http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy File .clang-tidy: http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy@67 PS6, Line 67: -clang-diagnostic-unreachable-code-break,\ > Might be nice to briefly mention the rationale for this in the commit messa I ended up changing the code to remove the extra break calls. This seemed consistent with what we did elsewhere, so I've removed this. http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc File be/src/runtime/io/data-cache.cc: http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc@556 PS6, Line 556: Cache::UniqueHandle handle( > nit: this will fit on a single line now, here and I think a couple of other Done -- To view, visit http://gerrit.cloudera.org:8080/15170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I917f8352c9276373dd2761af986bf3487855271c Gerrit-Change-Number: 15170 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 19 Feb 2020 01:47:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/15169 )
Change subject: IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache
..
Patch Set 7:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache-bench.cc
File be/src/util/cache/cache-bench.cc:
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache-bench.cc@176
PS7, Line 176: pair hits_lookups =
RunQueryThreads(FLAGS_num_threads, FLAGS_run_seconds);
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache-bench.cc@183
PS7, Line 183: LOG(INFO) << test_case << ": " <<
HumanReadableNum::ToString(l_per_sec) << " lookups/sec";
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache-bench.cc@184
PS7, Line 184: LOG(INFO) << test_case << ": " << StringPrintf("%.1f",
hit_rate * 100.0) << "% hit rate";
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache.h
File be/src/util/cache/cache.h:
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache.h@127
PS7, Line 127: // Passing EXPECT_IN_CACHE will increment the hit/miss metrics
that track the number of times
line too long (95 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache.h@128
PS7, Line 128: // blocks were requested that the users were hoping to get the
block from the cache, along with
line too long (97 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/cache.h@131
PS7, Line 131: // This helps in determining if we are effectively caching the
blocks that matter the most.
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/nvm_cache.cc
File be/src/util/cache/nvm_cache.cc:
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/nvm_cache.cc@8
PS7, Line 8: // This file implements a cache based on the MEMKIND library
(http://memkind.github.io/memkind/)
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/nvm_cache.cc@9
PS7, Line 9: // This library makes it easy to program against persistent memory
hardware by exposing an API
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/15169/7/be/src/util/cache/nvm_cache.cc@706
PS7, Line 706: int err = CALL_MEMKIND(memkind_create_pmem,
FLAGS_nvm_cache_path.c_str(), capacity, );
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/15169
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76
Gerrit-Change-Number: 15169
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Feb 2020 01:46:49 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache
Joe McDonnell has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/15169 ) Change subject: IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache .. IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache The data cache uses Kudu's cache code in be/src/kudu/util. To make it easy to make Impala specific changes (such as introducing new cache eviction algorithms), this copies the code into Impala's utilities directory. To make the review easier, this is a simple copy of the files with a minimal CMakeLists.txt and RAT changes. This does not link the resulting library into anything. A subsequent change will modify the code to be usable by Impala. To test, I verified that this compiles. Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76 --- A be/src/util/cache/CMakeLists.txt A be/src/util/cache/cache-bench.cc A be/src/util/cache/cache-test.cc A be/src/util/cache/cache.cc A be/src/util/cache/cache.h A be/src/util/cache/nvm_cache.cc A be/src/util/cache/nvm_cache.h M bin/rat_exclude_files.txt 8 files changed, 2,595 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/15169/7 -- To view, visit http://gerrit.cloudera.org:8080/15169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76 Gerrit-Change-Number: 15169 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15169 ) Change subject: IMPALA-8690 (prep 1): Copy Kudu cache code to be/src/util/cache .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/15169/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15169/6//COMMIT_MSG@7 PS6, Line 7: IMPALA-8960 > IMPALA-8690 Done -- To view, visit http://gerrit.cloudera.org:8080/15169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76 Gerrit-Change-Number: 15169 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:46:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5358/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Gerrit-Change-Number: 15202 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:38:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. Patch Set 5: Code-Review+2 This is a step in the right direction. I'm all for header cleanup. In a future change, we should check in whatever IWYU scripts would help take this farther. -- To view, visit http://gerrit.cloudera.org:8080/15202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Gerrit-Change-Number: 15202 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:37:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9373: Trial run of include-what-you-use
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15202 ) Change subject: IMPALA-9373: Trial run of include-what-you-use .. Patch Set 5: This is ready for review -- To view, visit http://gerrit.cloudera.org:8080/15202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3450e0ffcb8b183e18ac59c8b33b9ecbd3f60e20 Gerrit-Change-Number: 15202 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Feb 2020 01:16:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5260/ : 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/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 5 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Feb 2020 23:07:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. Patch Set 5: Hi all, please ignore this patch set since I will push another one with a newer CDH_BUILD_NUMBER. Sorry for the confusion caused. -- To view, visit http://gerrit.cloudera.org:8080/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 5 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Feb 2020 22:32:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8712: Make ExecQueryFInstances async
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15154 ) Change subject: IMPALA-8712: Make ExecQueryFInstances async .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator-backend-state.cc@195 PS5, Line 195: rpc_latency_ > I'm not sure you're correct about this. One observation is that I haven't a So after talking with Tim about this offline and doing some additional reading (eg. https://en.cppreference.com/w/cpp/language/memory_model) I'm confident that its thread-safe as is. The reason is that the read and write are synchronized by blocking/notifying on the CountingBarrier, which results in a memory barrier. -- To view, visit http://gerrit.cloudera.org:8080/15154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33ec96e5885af094c294cd3a76c242995263ba32 Gerrit-Change-Number: 15154 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 22:25:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre
Fang-Yu Rao has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15214 ) Change subject: IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre .. IMPALA-8870: Bump up guava version from 14.0.1 to 28.1-jre This patch bumps up the version of Guava libraries from 14.0.1 to 28.1-jre. Due to some changes in Guava's API's, we modify the call sites accordingly. Note that in order to instruct the Java classes under the directory of $IMPALA_HOME/common/yarn-extras to use the new Guava libraries, we also explicitly added a dependency in the corresponding pom.xml file. In addition, in this patch we bumped up CDH_BUILD_NUMBER to 1893106 so as to use the Sentry compiled with the newer Guava libraries, allowing Impala to perform authorization via Sentry. Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 --- M bin/impala-config.sh M common/yarn-extras/pom.xml M common/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.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/IsNullPredicate.java M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Column.java M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java M fe/src/main/java/org/apache/impala/catalog/DataSource.java M fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/TableLoader.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java M fe/src/main/java/org/apache/impala/planner/DataPartition.java M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java M fe/src/main/java/org/apache/impala/planner/ScanNode.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java M impala-parent/pom.xml 49 files changed, 105 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/15214/5 -- To view, visit http://gerrit.cloudera.org:8080/15214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9690a926953a8d3c3872277680b4be0551546c68 Gerrit-Change-Number: 15214 Gerrit-PatchSet: 5 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9383: Fix hang in hs2-http server with large, chunks request
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15218 ) Change subject: IMPALA-9383: Fix hang in hs2-http server with large, chunks request .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Gerrit-Change-Number: 15218 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 22:18:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9383: Fix hang in hs2-http server with large, chunks request
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15218 ) Change subject: IMPALA-9383: Fix hang in hs2-http server with large, chunks request .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5357/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Gerrit-Change-Number: 15218 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 22:18:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9383: Fix hang in hs2-http server with large, chunks request
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15218 ) Change subject: IMPALA-9383: Fix hang in hs2-http server with large, chunks request .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15218 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If822fb9cd2d7f2b0f5e36879fd7ccbba9a217ad0 Gerrit-Change-Number: 15218 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 22:16:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8712: Make ExecQueryFInstances async
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15154 ) Change subject: IMPALA-8712: Make ExecQueryFInstances async .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5259/ : 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/15154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33ec96e5885af094c294cd3a76c242995263ba32 Gerrit-Change-Number: 15154 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 21:44:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5258/ : 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/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 18 Feb 2020 21:11:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8712: Make ExecQueryFInstances async
Hello Sahil Takiar, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15154 to look at the new patch set (#6). Change subject: IMPALA-8712: Make ExecQueryFInstances async .. IMPALA-8712: Make ExecQueryFInstances async This patch refactors the ExecQueryFInstances rpc to be asychronous. Previously, Impala would issue all the Exec()s, wait for all of them to complete, and then check if any of them resulted in an error. We now stop issuing Exec()s and cancel any that are still in flight as soon as an error occurs. It also performs some cleanup around the thread safety of Coordinator::BackendState, including adding comments and DCHECKS. It also adds a new profile timer, ExecSerializationTimer, which records the amount of time spent perparing the Exec() params for each backend. === Exec RPC Thread Pool === This patch also removes the 'exec_rpc_thread_pool_' from ExecEnv. This thread pool was used to partially simulate async Exec() prior to the switch to KRPC, which provides built-in async rpc capabilities. Removing this thread pool has potential performance implications, as it means that the Exec() parameters are serialized in serialize rather than in parallel (with the level of parallelism determined by the size of the thread pool, which was configurable by an Advanced flag and defaulted to 12). To ensure we don't regress query startup times, I did some performance testing. All tests were done on a 10 node cluster. The baseline used for the tests did not include IMPALA-9181, a perf optimization for query startup done to facilitate this work. I ran TPCH 100 at concurrency levels of 1, 4, and 8 and extracted the query startup times from the profiles. For each concurrency level, the average regression in query startup time was < 2ms. Because query e2e running time was much longer than this, there was no noticable change in total query time. I also ran a 'worst case scenario' with a table with 10,000 pertitions to create a very large Exec() payload to serialize (~1.21MB vs. ~10KB-30KB for TPCH 100). Again, change in query startup time was neglible. Testing: - Added a e2e test that verifies that a query where an Exec() fails doesn't wait for all Exec()s to complete before cancelling and returning the error to the client. Change-Id: I33ec96e5885af094c294cd3a76c242995263ba32 --- M be/src/common/global-flags.cc 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/query-state.cc M be/src/util/counting-barrier.h M tests/custom_cluster/test_rpc_exception.py M tests/failure/test_failpoints.py 11 files changed, 444 insertions(+), 266 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/15154/6 -- To view, visit http://gerrit.cloudera.org:8080/15154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I33ec96e5885af094c294cd3a76c242995263ba32 Gerrit-Change-Number: 15154 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8712: Make ExecQueryFInstances async
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15154 ) Change subject: IMPALA-8712: Make ExecQueryFInstances async .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator-backend-state.cc@195 PS5, Line 195: rpc_latency_ > I'm not sure if this variable is thread safe anymore - I think this Cb is b I'm not sure you're correct about this. One observation is that I haven't actually changed the thread safety of these variables - previously they would have been written in exec_rpc_thread_pool_ and read by a different thread. I'll also note that the article you previously linked to (http://igoro.com/archive/volatile-keyword-in-c-memory-model-explained/) seems to say this isn't an issue for x86, which afaik Impala is only ever run on x86, though I'm not necessarily thrilled about relying on that. Its probably not a big deal to just go ahead and hold 'lock_' when these are read, since they're only read in FinishBackendStartup() and it won't really add any lock contention. However, I am curious about this, so I'm gonna try to ask around to see if there's someone more knowledgeable who can weigh in. http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator-backend-state.cc@227 PS5, Line 227: DCHECK(!exec_done_); > move this to right before the ThriftSerializer is called? otherwise if the Sure, I moved it down to right before the call to SetRpcParams(), which I think makes sense to include in this timer as part of the work to set up the rpc params. If that still seems confusing, it might be necessary to rename the timer. http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator.cc@447 PS5, Line 447: ThriftSerializer serializer(true); > should SCOPED_TIMER(exec_serialization_timer_); go here as well? Sure, I can add one if you want, but I think this is less interesting to time than the other serialization stuff as it only happens once per query and the amount of data being serialized is generally much smaller. http://gerrit.cloudera.org:8080/#/c/15154/5/be/src/runtime/coordinator.cc@982 PS5, Line 982: DCHECK(exec_rpcs_complete_.Load()) << "Exec() must be called first."; > Will this break the ImpalaServer UnresponsiveBackendThread thread? It calls So its confusing and I don't know why they did it this way, but all of these calls are on a Coordinator object that is returned by ClientRequestState::GetCoordinator(), which returns nullptr if the coordinator hasn't successfully completed the call to Exec(), so no all of the DCHECKs are valid. -- To view, visit http://gerrit.cloudera.org:8080/15154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33ec96e5885af094c294cd3a76c242995263ba32 Gerrit-Change-Number: 15154 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 21:01:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 6: (10 comments) Thanks for the review! I fixed the ones I were sure about, will return to some of the more complex ones. http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG@13 PS6, Line 13: is was > was no ? Done http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG@26 PS6, Line 26: that > nit: not necessary Done http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/common/global-types.h File be/src/common/global-types.h: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/common/global-types.h@31 PS6, Line 31: #define UTCPTR nullptr > Why no define a const Timezone* instead? It led to compile error for some reason. I will try it again, probably it should be const Timezone * const http://gerrit.cloudera.org:8080/#/c/15222/7/be/src/exec/data-source-scan-node.cc File be/src/exec/data-source-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/15222/7/be/src/exec/data-source-scan-node.cc@352 PS7, Line 352: // TODO The timezone depends on flag use_local_tz_for_unix_timestamp_conversions. : // Check if this is the intended behaviour. : RETURN_IF_ERROR(MaterializeNextRow( : state->time_zone_for_unix_time_conversions(), tuple_pool, tuple)); > You're raising a good point in the comment. I think here we should just pas I was thinking about UTCPTR instead. Using local_time_zone() would mean that we will do a timezone conversion by default, which is not how Impala used to work. Note that I am not sure whether anyone cares about the correctness of DataSource. http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/exprs/expr-test.cc@170 PS6, Line 170: const Timezone& > Returning const Timezone* here might simplify things a bit. This cannot return nullptr, as it is also used in UtcToLocal() which expects a Timezone reference, so I think that it is clearer to keep it this way. http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/runtime/timestamp-value.h File be/src/runtime/timestamp-value.h: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/runtime/timestamp-value.h@102 PS6, Line 102: 'unix_time' is assumed to be UTC > nit: The comment is a bit confusing. 'unix_time' is always in UTC (by defin I didn't find a good name for this offset when used in timezone agnostic context. E.g. both Parquet and Orc store such offsets that behave exactly like Unix time, while assume the resulting timestamp to be in local time. We consider this the "normal" case and pass a pointer to a Timezone when the unix_time is real UTC Unix time and we need to convert it to local time. http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test File testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test: http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test@3 PS6, Line 3: This test is also called with convert_legacy_hive_parquet_utc_timestamps=true. > Comment is confusing: I think the test is only called with convert_legacy_h You are right, I confused this test with another one. http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test File testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test: http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test@18 PS6, Line 18: : QUERY > Move the new sections to a separate .test file or rename this file to somet Done http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test@45 PS6, Line 45: QUERY : SET timezone=CET; : select min(timestamp_col) from functional_avro.alltypestiny; : TYPES : STRING : RESULTS : '2009-01-01 00:00:00' > Since functional_avro.alltypestiny.timestamp_col is a string so probably yo I kept the test - if we add timestamp support for avro one day, it will be easy to forget about checking this. http://gerrit.cloudera.org:8080/#/c/15222/6/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py File tests/custom_cluster/test_hive_parquet_timestamp_conversion.py: http://gerrit.cloudera.org:8080/#/c/15222/6/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@74 PS6, Line 74:
[Impala-ASF-CR] IMPALA-8755: Backend support for Z-ordering
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 ) Change subject: IMPALA-8755: Backend support for Z-ordering .. Patch Set 27: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5356/ -- To view, visit http://gerrit.cloudera.org:8080/14080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0200748ce3e65ebc5d3530f794c0f80aa335a2ab Gerrit-Change-Number: 14080 Gerrit-PatchSet: 27 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Anonymous Coward (520) Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 20:29:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Hello Quanlong Huang, Norbert Luksa, Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15222 to look at the new patch set (#8). Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. IMPALA-9385: Unix time conversion cleanup + ORC fix Orc scanner uses TimestampValue::FromUnixTimeNanos() to convert sec + nano representation to Impala's TimestampValue (day + nano). FromUnixTimeNanos was affected by flag use_local_tz_for_unix_timestamp_conversions, while that global option should not affect ORC. By default there was no conversion, but if the flag is 1, then timestamps were interpreted as UTC and converted to local time. This could be solved by creating a UTC version of FromUnixTimeNanos, but I decided to change the interface in the hope of making To/From timestamp functions less confusing. Changes: - Fixed the bug by passing UTC as timezone in the ORC scanner. - Changed the interface of these TimestampValue functions to expect a timezone pointer, interpret null as UTC and skip conversion. It would be also possible to pass the actual UTC timezone and check for this in the functions, but I guess it is easier to optimize the inlined functions this way. - Moved the checking of use_local_tz_for_unix_timestamp_conversions to RuntimeState and added property time_zone_for_unix_time_conversions() to return the timezone to use in Unix time conversions. This made TimestampValue's interface clearer and makes it easy to replace the flag with a query option if we want to. - Changed RuntimeState and the Parquet scanner to skip timezone conversion if convert_legacy_hive_parquet_utc_timestamps=1 but the timezone is UTC. This allows users to avoid the performance penalty of this flag by setting query option timezone to UTC in their session (IMPALA-7557). CCTZ is not good at this, actually conversions are slower with fixed offset timezones (including UTC) than with timezones that have DST/historical rule changes. Postponed changes: - Didn't remove the UTC versions of the functions yet, as that would require changing (and possibly rethinking) several BE tests and benchmarks. Tests: - Added regression test for Orc and other file formats to check that they are not affected by this flag. - Extended test_hive_parquet_timestamp_conversion.py to cover the case when convert_legacy_hive_parquet_utc_timestamps=1 and timezone=UTC. Also did some cleanup there to use query option timezone instead of env var TZ. Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e --- M be/src/benchmarks/convert-timestamp-benchmark.cc M be/src/common/global-types.h M be/src/exec/data-source-scan-node.cc M be/src/exec/data-source-scan-node.h M be/src/exec/orc-column-readers.cc M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/parquet-common.h M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/cast-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc M be/src/exprs/timezone_db.h M be/src/runtime/raw-value-test.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/timestamp-test.cc M be/src/runtime/timestamp-value.cc M be/src/runtime/timestamp-value.h M be/src/runtime/timestamp-value.inline.h M testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test M testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py M tests/custom_cluster/test_local_tz_conversion.py 26 files changed, 218 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/15222/8 -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] WIP: IMPALA-9156: share broadcast join builds
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15096 ) Change subject: WIP: IMPALA-9156: share broadcast join builds .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5257/ : 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/15096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c67e4b2c87ed0fba648f1e1710addb885d66dc7 Gerrit-Change-Number: 15096 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 18 Feb 2020 19:10:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 3): Factor out common code for cache implementations
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15179 ) Change subject: IMPALA-8690 (prep 3): Factor out common code for cache implementations .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/15179/5/be/src/util/cache/cache-internal.h File be/src/util/cache/cache-internal.h: http://gerrit.cloudera.org:8080/#/c/15179/5/be/src/util/cache/cache-internal.h@269 PS5, Line 269: HandleType* h = reinterpret_cast(pending_handle); : delete h; I ran ASAN, and it is unhappy about the asymmetry between using a new to allocate an array and then using a regular delete on this. I'm fixing it to make it symmetric (i.e. call destructor and then call delete []). -- To view, visit http://gerrit.cloudera.org:8080/15179 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67294244a3e8a2812f1482fe786bf7f8e6ce054e Gerrit-Change-Number: 15179 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 19:01:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4224: execute separate join builds fragments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14859 ) Change subject: IMPALA-4224: execute separate join builds fragments .. Patch Set 41: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5256/ : 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/14859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4403c8e62d9c13854e7830602ee613f8efc80c58 Gerrit-Change-Number: 14859 Gerrit-PatchSet: 41 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 18:56:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15170 ) Change subject: IMPALA-8690 (prep 2): Switch DataCache to Impala's cache implementation .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15170/6//COMMIT_MSG@16 PS6, Line 16: 3. Remove CacheMetrics code (not used by Impala) I assume we have some other way of tracking these metrics? Or are they not important to us? http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy File .clang-tidy: http://gerrit.cloudera.org:8080/#/c/15170/6/.clang-tidy@67 PS6, Line 67: -clang-diagnostic-unreachable-code-break,\ Might be nice to briefly mention the rationale for this in the commit message. http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc File be/src/runtime/io/data-cache.cc: http://gerrit.cloudera.org:8080/#/c/15170/6/be/src/runtime/io/data-cache.cc@556 PS6, Line 556: Cache::UniqueHandle handle( nit: this will fit on a single line now, here and I think a couple of other places in this file -- To view, visit http://gerrit.cloudera.org:8080/15170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I917f8352c9276373dd2761af986bf3487855271c Gerrit-Change-Number: 15170 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 18:55:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8960 (prep 1): Copy Kudu cache code to be/src/util/cache
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15169 ) Change subject: IMPALA-8960 (prep 1): Copy Kudu cache code to be/src/util/cache .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/15169/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15169/6//COMMIT_MSG@7 PS6, Line 7: IMPALA-8960 IMPALA-8690 -- To view, visit http://gerrit.cloudera.org:8080/15169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7b8b852b56613803dea125456fa9e8b3736d76 Gerrit-Change-Number: 15169 Gerrit-PatchSet: 6 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 18:37:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 7: (10 comments) http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG@13 PS6, Line 13: is was was no ? http://gerrit.cloudera.org:8080/#/c/15222/6//COMMIT_MSG@26 PS6, Line 26: that nit: not necessary http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/common/global-types.h File be/src/common/global-types.h: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/common/global-types.h@31 PS6, Line 31: #define UTCPTR nullptr Why no define a const Timezone* instead? http://gerrit.cloudera.org:8080/#/c/15222/7/be/src/exec/data-source-scan-node.cc File be/src/exec/data-source-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/15222/7/be/src/exec/data-source-scan-node.cc@352 PS7, Line 352: // TODO The timezone depends on flag use_local_tz_for_unix_timestamp_conversions. : // Check if this is the intended behaviour. : RETURN_IF_ERROR(MaterializeNextRow( : state->time_zone_for_unix_time_conversions(), tuple_pool, tuple)); You're raising a good point in the comment. I think here we should just pass state->local_time_zone(). http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/exprs/expr-test.cc@170 PS6, Line 170: Returning const Timezone* here might simplify things a bit. http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/runtime/timestamp-value.h File be/src/runtime/timestamp-value.h: http://gerrit.cloudera.org:8080/#/c/15222/6/be/src/runtime/timestamp-value.h@102 PS6, Line 102: 'unix_time' is assumed to be UTC nit: The comment is a bit confusing. 'unix_time' is always in UTC (by definition) not just when 'local_tz' is set to non-UTC. http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test File testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test: http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test@3 PS6, Line 3: This test is also called with convert_legacy_hive_parquet_utc_timestamps=true. Comment is confusing: I think the test is only called with convert_legacy_hive_parquet_utc_timestamps=true. http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test File testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test: http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test@18 PS6, Line 18: : QUERY Move the new sections to a separate .test file or rename this file to something more appropriate. Originally this test file was meant for testing UTC timestamp functions only. http://gerrit.cloudera.org:8080/#/c/15222/6/testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test@45 PS6, Line 45: QUERY : SET timezone=CET; : select min(timestamp_col) from functional_avro.alltypestiny; : TYPES : STRING : RESULTS : '2009-01-01 00:00:00' Since functional_avro.alltypestiny.timestamp_col is a string so probably you can remove this section. http://gerrit.cloudera.org:8080/#/c/15222/6/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py File tests/custom_cluster/test_hive_parquet_timestamp_conversion.py: http://gerrit.cloudera.org:8080/#/c/15222/6/tests/custom_cluster/test_hive_parquet_timestamp_conversion.py@74 PS6, Line 74: self.check_sanity(True) : # Test with UTC too to check the optimizations added in IMPALA-9385. : for tz_name in ["PST8PDT", "UTC"]: : # The value read from the Hive table should be the same as reading a UTC converted : # value from the Impala table. : data = self.execute_query_expect_success(self.client, """ : SELECT h.id, h.day, h.timestamp_col, i.timestamp_col : FROM functional_parquet.alltypesagg_hive_13_1 h : JOIN functional_parquet.alltypesagg : i ON i.id = h.id AND i.day = h.day -- serves as a unique key : WHERE : (h.timestamp_col IS NULL AND i.timestamp_col IS NOT NULL) : OR (h.timestamp_col IS NOT NULL AND i.timestamp_col IS NULL) : OR h.timestamp_col !=
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15237 )
Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
..
Patch Set 1:
(2 comments)
I think this looks good, we need some minor cleanup before merging
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh@220
PS1, Line 220: `uname -p`
Can you use $(uname -p) to be consistent with the rest of the script?
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh@297
PS1, Line 297: if [[ $ARCH_NAME == 'aarch64' ]]; then
I think you can simplify this to:
SET_LD_LIBRARY_PATH='export
LD_LIBRARY_PATH=/usr/lib/${ARCH_NAME}-linux-gnu/${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}'
--
To view, visit http://gerrit.cloudera.org:8080/15237
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf
Gerrit-Change-Number: 15237
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 18 Feb 2020 18:32:46 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8800: Added support of Kudu DATE type to Impala
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14705 ) Change subject: IMPALA-8800: Added support of Kudu DATE type to Impala .. Patch Set 13: In case you weren't aware, the patch that's needed to get this working with proper Kudu DATE support instead of relying on building Impala against a local Kudu build has gone in: https://gerrit.cloudera.org/#/c/15134/ I think you'll still need to bump the version of Kudu in native-toolchain, as its currently at 5c610bf40 which doesn't include the Java client changes, but that's easy. If you don't know how to do that, feel free to reach out to me. -- To view, visit http://gerrit.cloudera.org:8080/14705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91656749a58ac769b54c2a63bdd4f85c89520b32 Gerrit-Change-Number: 14705 Gerrit-PatchSet: 13 Gerrit-Owner: Volodymyr Verovkin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Comment-Date: Tue, 18 Feb 2020 18:31:18 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP: IMPALA-9156: share broadcast join builds
Tim Armstrong has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15096 ) Change subject: WIP: IMPALA-9156: share broadcast join builds .. WIP: IMPALA-9156: share broadcast join builds The scheduler will only create one join build finstance per backend in cases where this is supported. The builder is aware of the number of finstances executing the probe and hands off the build data structures to the builders. Nested loop join requires minimal modifications because the build data structures are read-only after initial construction. The only significant change is that memory can't be transferred to the multiple consumers, so MarkNeedsDeepCopy() needs to be used instead. Hash join requires additional synchronisation because the spilling algorithm mutates build-side data structures. This patch adds synchronisation so that rebuilding spilled partitions is done in a thread-safe manner, using a single thread. This uses the CyclicBarrier added in an earlier patch. Threads blocked on CyclicBarrier need to be cancellable, which is handled by cancelling the barrier when closing the join builder. Update planner to cost broadcast join and estimate memory consumption based on a single instance per node. Planner estimates of number of instances are improved. Instead of assuming mt_dop instances per node, use the total number of input splits (also called scan ranges in places) as an upper bound on the number of instances generated by scans. These instance estimates from the scan nodes are then propagated up the plan tree in the same was as the numNodes estimates. TODO: * Clean up hack in join build resource estimation * Decide whether to retain MarkNeedsDeepCopy() or actually transfer buffers with shared ownership * Is timing correct for embedded builder case? * Clean up locking around the reservation transfer Testing: TODO: * Update/check planner tests * Stress testing Change-Id: I4c67e4b2c87ed0fba648f1e1710addb885d66dc7 --- M be/src/exec/join-builder.cc M be/src/exec/join-builder.h M be/src/exec/join-op.h M be/src/exec/nested-loop-join-node.cc M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M be/src/util/cyclic-barrier-test.cc M be/src/util/cyclic-barrier.h M common/thrift/DataSinks.thrift M common/thrift/ImpalaInternalService.thrift M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/EmptySetNode.java M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M fe/src/main/java/org/apache/impala/planner/PlanFragment.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/SingularRowSrcNode.java M fe/src/main/java/org/apache/impala/planner/UnionNode.java M fe/src/main/java/org/apache/impala/planner/UnnestNode.java 34 files changed, 396 insertions(+), 144 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/15096/5 -- To view, visit http://gerrit.cloudera.org:8080/15096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c67e4b2c87ed0fba648f1e1710addb885d66dc7 Gerrit-Change-Number: 15096 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 ) Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output .. Patch Set 2: Yeah, its definitely pretty weird to me that "show table stats" shows partition information for Kudu tables but not for HDFS tables. Seems like that's what "show partitions" is for, and "show table stats" should be for table-level info. So I would say - for "show table stats" only show stuff that's table-level and that we have info for, so definitely "# rows", probably a "format -> KUDU" column like how HDFS has "format->TEXT/PARQUET/whatever", and maybe stuff like "# partitions" or "size" if those things are available. And then for "show partitions" don't add the "total # rows", just exclude the "# rows" column entirely. -- To view, visit http://gerrit.cloudera.org:8080/15199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041 Gerrit-Change-Number: 15199 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 18:21:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4224: execute separate join builds fragments
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14859 )
Change subject: IMPALA-4224: execute separate join builds fragments
..
Patch Set 41:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14859/38/fe/src/main/java/org/apache/impala/planner/PlanFragment.java
File fe/src/main/java/org/apache/impala/planner/PlanFragment.java:
http://gerrit.cloudera.org:8080/#/c/14859/38/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@304
PS38, Line 304: Preconditions.checkArgument(perInstanceInitialMemReservatio
> Check in the loops looks a bit weird to me - I would prefer to check it out
Done
http://gerrit.cloudera.org:8080/#/c/14859/38/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@407
PS38, Line 407: result = -1;
: break;
: }
: if (dataPartition_.getPartitionExprs().contains(expr)) {
: numDistinct = (long)Math.max((double) numDistinct /
(double) numInstances, 1L);
: }
> I think that this is not the best place for these checks - we combine two i
I generally like the pattern of integrity checks right before serialising to
thrift because it catches anything malformed before we send it to the backend.
Doing it earlier makes it easier to accidentally bypass the checks, e.g. with
an early return. I'll factor these out into a function and also call it in
computeResourceProfile so that there's less unrelated noise in this function.
I updated the comment. It was replaced by finalizeExchanges() in
http://gerrit.cloudera.org:8080/7223. Looks like I missed updating the comment
back then. Made a few other improvements to the class comment.
I removed the TODO about the PlanNode trees because I'm not sure that it's
something we actually want to do
http://gerrit.cloudera.org:8080/#/c/14859/40/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:
http://gerrit.cloudera.org:8080/#/c/14859/40/fe/src/main/java/org/apache/impala/planner/PlanNode.java@476
PS40, Line 476: // ExchangeNodes and separated join builds) to have no
children.
: int numChildren = 0;
: for (PlanNode child: children_) {
: if (child.getFragment() != getFragment()) continue;
: child.treeToThriftHelper(container);
: ++numChildren;
: }
: msg.num_children = numChildren;
: }
:
: /**
:* Computes the full internal state, including smap and
planner-relevant statistics
:* (calls computeStats()), marks all slots referenced by this
node as materialized
:* and computes the mem layout of all materialized tuples
(with the assumption that
:* slots that are needed by ancestor PlanNodes have already
been marked).
:*
> Can't we generalize this by checking if fragment_ is the same as in the chi
This is a nice simplification, thanks.
http://gerrit.cloudera.org:8080/#/c/14859/40/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java
File fe/src/main/java/org/apache/impala/planner/ResourceProfile.java:
http://gerrit.cloudera.org:8080/#/c/14859/40/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java@157
PS40, Line 157: combine
> Is sum() still useful after creating combine? Are there case where specific
Yeah, sum() makes sense in cases where you're adding together the resource
requirements but having buffer sizes in the resulting profile doesn't make
sense. E.g. if are adding up PlanNode resources to compute fragment-level
resources - each node has it's own independent buffer size so showing a buffer
size at the fragment level doesn't make sense.
I originally did not have these separate, but it actually added a bunch of
noise in explain plans.
http://gerrit.cloudera.org:8080/#/c/14859/40/fe/src/main/java/org/apache/impala/planner/ResourceProfile.java@172
PS40, Line 172: invalid values
> We return invalid instead of max if I understand correctly
Good point
--
To view, visit http://gerrit.cloudera.org:8080/14859
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4403c8e62d9c13854e7830602ee613f8efc80c58
Gerrit-Change-Number: 14859
Gerrit-PatchSet: 41
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 18 Feb 2020 18:14:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4224: execute separate join builds fragments
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14859 ) Change subject: IMPALA-4224: execute separate join builds fragments .. Patch Set 41: Code-Review+1 carry +1 -- To view, visit http://gerrit.cloudera.org:8080/14859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4403c8e62d9c13854e7830602ee613f8efc80c58 Gerrit-Change-Number: 14859 Gerrit-PatchSet: 41 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 18:14:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4224: execute separate join builds fragments
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14859 to look at the new patch set (#41). Change subject: IMPALA-4224: execute separate join builds fragments .. IMPALA-4224: execute separate join builds fragments This enables parallel plans with the join build in a separate fragment and fixes all of the ensuing fallout. After this change, mt_dop plans with joins have separate build fragments. There is still a 1:1 relationship between join nodes and builders, so the builders are only accessed by the join node's thread after it is handed off. This lets us defer the work required to make PhjBuilder and NljBuilder safe to be shared between nodes. Planner changes: * Combined the parallel and distributed planning code paths. * Misc fixes to generate reasonable thrift structures in the query exec requests, i.e. containing the right nodes. * Fixes to resource calculations for the separate build plans. ** Calculate separate join/build resource consumption. ** Simplified the resource estimation by calculating resource consumption for each fragment separately, and assuming that all fragments hit their peak resource consumption at the same time. IMPALA-9255 is the follow-on to make the resource estimation more accurate. Scheduler changes: * Various fixes to handle multiple TPlanExecInfos correctly, which are generated by the planner for the different cohorts. * Add logic to colocate build fragments with parent fragments. Runtime filter changes: * Build sinks now produce runtime filters, which required planner and coordinator fixes to handle. DataSink changes: * Close the input plan tree before calling FlushFinal() to release resources. This depends on Send() not holding onto references to input batches, which was true except for NljBuilder. This invariant is documented. Join builder changes: * Add a common base class for PhjBuilder and NljBuilder with functions to handle synchronisation with the join node. * Close plan tree earlier in FragmentInstanceState::Exec() so that peak resource requirements are lower. * The NLJ always copies input batches, so that it can close its input tree. JoinNode changes: * Join node blocks waiting for build-side to be ready, then eventually signals that it's done, allowing the builder to be cleaned up. * NLJ and PHJ nodes handle both the integrated builder and the external builder. There is a 1:1 relationship between the node and the builder, so we don't deal with thread safety yet. * Buffer reservations are transferred between the builder and join node when running with the separate builder. This is not really necessary right now, since it is all single-threaded, but will be important for the shared broadcast. - The builder transfers memory for probe buffers to the join node at the end of each build phase. - At end of each probe phase, reservation needs to be handed back to builder (or released). ExecSummary changes: * The summary logic was modified to handle connecting fragments via join builds. The logic is an extension of what was used for exchanges. Testing: * Enable --unlock_mt_dop for end-to-end tests * Migrate some tests to run as part of end-to-end tests instead of custom cluster. * Add mt_dop dimension to various end-to-end tests to provide coverage of join queries, spill-to-disk and cancellation. * Ran a single node TPC-H and TPC-DS stress test with mt_dop=0 and mt_dop=4. Perf: * Ran TPC-H scale factor 30 locally with mt_dop=0. No significant change. Change-Id: I4403c8e62d9c13854e7830602ee613f8efc80c58 --- M be/src/exec/CMakeLists.txt M be/src/exec/blocking-join-node.cc M be/src/exec/blocking-join-node.h M be/src/exec/data-sink.cc M be/src/exec/data-sink.h M be/src/exec/exec-node.h A be/src/exec/join-builder.cc A be/src/exec/join-builder.h M be/src/exec/nested-loop-join-builder.cc M be/src/exec/nested-loop-join-builder.h M be/src/exec/nested-loop-join-node.cc M be/src/exec/nested-loop-join-node.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool-internal.h M be/src/runtime/bufferpool/buffer-pool-test.cc M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/initial-reservations.cc M be/src/runtime/row-batch.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/spillable-row-batch-queue.h M be/src/util/summary-util.cc M bin/run-all-tests.sh M common/thrift/DataSinks.thrift M
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5255/ : 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/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 18 Feb 2020 17:10:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9366: Remove embedded pointer reference in PhjBuilder::CodegenInsertRuntimeFilters
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15186 ) Change subject: IMPALA-9366: Remove embedded pointer reference in PhjBuilder::CodegenInsertRuntimeFilters .. Patch Set 3: fyi the reason for the failed build seems to be an unrelated flaky test: IMPALA-9397 -- To view, visit http://gerrit.cloudera.org:8080/15186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09037b5832b037536bde9324f7a2a1077854 Gerrit-Change-Number: 15186 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 16:44:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5092 Add support for VARCHAR in Kudu tables
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14197 ) Change subject: IMPALA-5092 Add support for VARCHAR in Kudu tables .. Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@a2217 PS10, Line 2217: Should this be move to AnalyzesOk just below the decimal one above? http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@344 PS10, Line 344: "valvarchar varchar(10) default null) " + nit: Maybe call it `valvc` to follow the pattern of the other columns. It might also be a bit easier to read. http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test File testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test: http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test@7 PS10, Line 7:valdec16 decimal(38, 0) null, valvarchar varchar(500)) Any reason you use 500 here and 10 in the insert test? http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test File testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test: http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test@7 PS10, Line 7:decimal16 decimal(38, 0) null, valvarchar varchar(10) null) Do any of these tests validate truncation behavior? -- To view, visit http://gerrit.cloudera.org:8080/14197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d4959410fdd882bfa980cb55e8a7837c7823da8 Gerrit-Change-Number: 14197 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 18 Feb 2020 16:33:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8852: Skip short-circuit config check for dedicated coordinator
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skip short-circuit config check for dedicated coordinator .. Patch Set 8: Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5355/ -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 18 Feb 2020 16:26:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Backend support for Z-ordering
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 ) Change subject: IMPALA-8755: Backend support for Z-ordering .. Patch Set 27: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0200748ce3e65ebc5d3530f794c0f80aa335a2ab Gerrit-Change-Number: 14080 Gerrit-PatchSet: 27 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Anonymous Coward (520) Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 16:06:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Backend support for Z-ordering
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 ) Change subject: IMPALA-8755: Backend support for Z-ordering .. Patch Set 27: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5356/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0200748ce3e65ebc5d3530f794c0f80aa335a2ab Gerrit-Change-Number: 14080 Gerrit-PatchSet: 27 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Anonymous Coward (520) Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 16:06:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 7: PS 7 fixed the clang tidy compile issue. What broke the last code review check is something unrelated: 00:22:31.237 [ERROR] Failed to execute goal on project impala-minimal-hive-exec: Could not resolve dependencies for project org.apache.impala:impala-minimal-hive-exec:jar:0.1-SNAPSHOT: Failed to collect dependencies at org.apache.hive:hive-exec:jar:2.1.1-cdh6.x-SNAPSHOT -> org.apache.calcite:calcite-core:jar:1.12.0 -> org.slf4j:slf4j-api:jar:1.7.13: Failed to read artifact descriptor for org.slf4j:slf4j-api:jar:1.7.13: Could not transfer artifact org.slf4j:slf4j-parent:pom:1.7.13 from/to impala.cdp.repo (https://native-toolchain.s3.amazonaws.com/build/cdp_components/1617729/maven): Access denied to: https://native-toolchain.s3.amazonaws.com/build/cdp_components/1617729/maven/org/slf4j/slf4j-parent/1.7.13/slf4j-parent-1.7.13.pom , ReasonPhrase:Forbidden. -> [Help 1] 00:22:31.237 [ERROR] -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 18 Feb 2020 15:56:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 ) Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output .. Patch Set 2: Hi Sahil, thank you for looking into this and adding Thomas. You mean removing all the columns and just returning the total number of rows? -- To view, visit http://gerrit.cloudera.org:8080/15199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041 Gerrit-Change-Number: 15199 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 18 Feb 2020 15:54:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9036: Fix CTRL+C a multiline query in impala-shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15233 ) Change subject: IMPALA-9036: Fix CTRL+C a multiline query in impala-shell .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5253/ : 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/15233 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8d8bdaee929e2655eb66e886ae92a02d3fbd83f Gerrit-Change-Number: 15233 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Tamas Gerrit-Reviewer: Adam Tamas Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Comment-Date: Tue, 18 Feb 2020 14:52:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 7: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/5254/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 18 Feb 2020 14:45:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15237 ) Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5252/ : 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/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 18 Feb 2020 14:30:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Hello Quanlong Huang, Norbert Luksa, Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15222 to look at the new patch set (#7). Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. IMPALA-9385: Unix time conversion cleanup + ORC fix Orc scanner uses TimestampValue::FromUnixTimeNanos() to convert sec + nano representation to Impala's TimestampValue (day + nano). FromUnixTimeNanos was affected by flag use_local_tz_for_unix_timestamp_conversions, while that global option should not affect ORC. By default there is was conversion, but if the flag is 1, then timestamps were interpreted as UTC and converted to local time. This could be solved by creating a UTC version of FromUnixTimeNanos, but I decided to change the interface in the hope of making To/From timestamp functions less confusing. Changes: - Fixed the bug by passing UTC as timezone in the ORC scanner. - Changed the interface of these TimestampValue functions to expect a timezone pointer, interpret null as UTC and skip conversion. It would be also possible to pass the actual UTC timezone and check for this in the functions, but I guess that it is easier to optimize the inlined functions this way. - Moved the checking of use_local_tz_for_unix_timestamp_conversions to RuntimeState and added property time_zone_for_unix_time_conversions() to return the timezone to use in Unix time conversions. This made TimestampValue's interface clearer and makes it easy to replace the flag with a query option if we want to. - Changed RuntimeState and the Parquet scanner to skip timezone conversion if convert_legacy_hive_parquet_utc_timestamps=1 but the timezone is UTC. This allows users to avoid the performance penalty of this flag by setting query option timezone to UTC in their session (IMPALA-7557). CCTZ is not good at this, actually conversions are slower with fixed offset timezones (including UTC) than with timezones that have DST/historical rule changes. Postponed changes: - Didn't remove the UTC versions of the functions yet, as that would require changing (and possibly rethinking) several BE tests and benchmarks. Tests: - Added regression test for Orc and other file formats to check that they are not affected by this flag. - Extended test_hive_parquet_timestamp_conversion.py to cover the case when convert_legacy_hive_parquet_utc_timestamps=1 and timezone=UTC. Also did some cleanup there to use query option timezone instead of env var TZ. Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e --- M be/src/benchmarks/convert-timestamp-benchmark.cc M be/src/common/global-types.h M be/src/exec/data-source-scan-node.cc M be/src/exec/data-source-scan-node.h M be/src/exec/orc-column-readers.cc M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/parquet-common.h M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/cast-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc M be/src/exprs/timezone_db.h M be/src/runtime/raw-value-test.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/timestamp-test.cc M be/src/runtime/timestamp-value.cc M be/src/runtime/timestamp-value.h M be/src/runtime/timestamp-value.inline.h M testdata/workloads/functional-query/queries/QueryTest/out-of-range-timestamp-local-tz-conversion.test M testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test M tests/custom_cluster/test_hive_parquet_timestamp_conversion.py 24 files changed, 280 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/15222/7 -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-9385: Unix time conversion cleanup + ORC fix
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 ) Change subject: IMPALA-9385: Unix time conversion cleanup + ORC fix .. Patch Set 6: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/5251/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 18 Feb 2020 14:18:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9036: Fix CTRL+C a multiline query in impala-shell
Adam Tamas has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15233 Change subject: IMPALA-9036: Fix CTRL+C a multiline query in impala-shell .. IMPALA-9036: Fix CTRL+C a multiline query in impala-shell Modified the `_signal_handler()` in impala-shell.py so when a user cancels a multiline query by hitting CTRL+C it will cancel the query, instead of just the current line. Testing: -Added test_cancellation_mid_command to test_shell_interactive.py to test if it really cancels the partial commands. -Manually tested by giving partial commands then cancelling them. Change-Id: Id8d8bdaee929e2655eb66e886ae92a02d3fbd83f --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 29 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/15233/2 -- To view, visit http://gerrit.cloudera.org:8080/15233 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id8d8bdaee929e2655eb66e886ae92a02d3fbd83f Gerrit-Change-Number: 15233 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Tamas Gerrit-Reviewer: Adam Tamas Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Norbert Luksa
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/15237 )
Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh@298
PS1, Line 298: SET_LD_LIBRARY_PATH='export
LD_LIBRARY_PATH=/usr/lib/aarch64-linux-gnu/${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}'
line too long (113 > 90)
http://gerrit.cloudera.org:8080/#/c/15237/1/bin/bootstrap_system.sh@300
PS1, Line 300: SET_LD_LIBRARY_PATH='export
LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu/${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}'
line too long (112 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/15237
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf
Gerrit-Change-Number: 15237
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 18 Feb 2020 13:47:06 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04
zhaoren...@hotmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15237 Change subject: IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 .. IMPALA-9396 Add arm64 openjdk version for Ubuntu 18.04 Add arm64 openjdk version for Ubuntu 18.04 in bootstrap_system.sh Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf --- M bin/bootstrap_system.sh 1 file changed, 14 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/15237/1 -- To view, visit http://gerrit.cloudera.org:8080/15237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8a7572439cef8f53499c3b20a09269613feea2cf Gerrit-Change-Number: 15237 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[Impala-ASF-CR] IMPALA-9392 Change Boost version for aarch64 supporting
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15231 ) Change subject: IMPALA-9392 Change Boost version for aarch64 supporting .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12f1d8fa1f3e35a62f3f42b3a2d19b85ca8c7a3d Gerrit-Change-Number: 15231 Gerrit-PatchSet: 6 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 12:57:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 ) Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06 Gerrit-Change-Number: 15235 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Feb 2020 13:08:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Hello Anurag Mantripragada, Fang-Yu Rao, Vihang Karajgaonkar, Csaba Ringhofer,
Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15235
to look at the new patch set (#4).
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
..
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework makes a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happen.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of CreatePolicy requests, so increases the possibility of
CreatePolicy requests running concurrently with GetPolicies requests
polling from other ranger plugin instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/4
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 ) Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06 Gerrit-Change-Number: 15235 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Feb 2020 12:52:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 ) Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1 Gerrit-Change-Number: 15047 Gerrit-PatchSet: 11 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Tue, 18 Feb 2020 12:01:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8852: Skip short-circuit config check for dedicated coordinator
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skip short-circuit config check for dedicated coordinator .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5355/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 8 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 18 Feb 2020 12:01:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15235 ) Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5250/ : 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/15235 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06 Gerrit-Change-Number: 15235 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 18 Feb 2020 11:43:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java:
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@30
PS2, Line 30: erImpalaPlugin ext
> As far as I know (I am not a java expert) INSTANCE should be voletile for d
Yes! We need volatile.
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@39
PS2, Line 39: public static RangerImpalaPlugin getInstance(String
serviceType, String appId) {
: if (INSTANCE == null) {
: synchronized(RangerImpalaPlugin.class) {
: if (INSTANCE == null) {
> nit: This could be skipped if there are two getInstance() calls in parallel
Nice find!
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Tue, 18 Feb 2020 11:02:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Hello Anurag Mantripragada, Fang-Yu Rao, Vihang Karajgaonkar, Csaba Ringhofer,
Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15235
to look at the new patch set (#3).
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
..
IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Fix the flakiness of Ranger FE tests in AuthorizationStmtTest which is
caused by a row filter policy not being cleanly deleted. There is a bug
in Ranger that policies being deleted in Ranger Admin server but still
exist in Ranger plugins when there are concurrent create policy and get
policy requests (RANGER-2727). It's more possible to hit the bug if we
have more ranger plugins running, since each plugin instance will poll
policies in each 30s regularly.
Impalad and Catalogd servers only initialize one ImpalaRangerPlugin
instance. However, AuthorizationStmtTest has embedded Frontend and
CatalogServiceCatalog objects. It will initialize two ranger plugin
instances totally. What's worse, the JUnit testing framework make a new
object for each test method run. Currently there are 29 test methods in
AuthorizationStmtTest, which means 29 AuthorizationStmtTest objects will
be created. So we finally have 58 ranger plugin instances running, which
makes RANGER-2727 easy to happens.
The failure can be reproduced by adding the following new test and run
it with all existing tests:
@Test
public void testRangerPolicyRepeatedly() throws Exception {
if (authzProvider_ == AuthorizationProvider.SENTRY) return;
for (int i = 0; i < 100; ++i) {
testRowFilterEnabled();
testColumnMaskEnabled();
}
}
We only explicitly create policies for column masking and row filtering
(other tests are using grant/revoke requests). This test increases the
number of create policy requests, so increases the possibility with
concurrent get policies requests polling from other ranger plugin
instances created by previous tests.
The fix is to make ImpalaRangerPlugin a singleton class so we will have
only one ranger plugin instance, which dramatically reduces the
possibility of hitting RANGER-2727. The thorough fix is bumping CDP
version after RANGER-2727 is resolved. Codes added in the previous patch
(c1244c2f04e629cc07b0830a597c70317be92768) are removed.
Tests:
- Ran AuthorizationStmtTest with the above new test.
Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
---
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
5 files changed, 39 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/15235/3
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9290: ORC scanner should support schema evolution between date and timestamp types
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/15152 ) Change subject: IMPALA-9290: ORC scanner should support schema evolution between date and timestamp types .. Patch Set 3: Regarding Csaba's concerns, I checked if Hive supports the same kind of schema evolution for Parquet, since we know that Impala doesn't. Found out that Hive doesn't support it either, only for ORC. -- To view, visit http://gerrit.cloudera.org:8080/15152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7979ecc61b2ab900090d01bc81e7bb7b28c99c9e Gerrit-Change-Number: 15152 Gerrit-PatchSet: 3 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 10:22:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/15235 )
Change subject: IMPALA-9152: Make Impala ranger plugin singleton in FE Tests
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java:
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@30
PS2, Line 30: RangerImpalaPlugin
As far as I know (I am not a java expert) INSTANCE should be voletile for
double-checked locking to work for singletons.
http://gerrit.cloudera.org:8080/#/c/15235/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaPlugin.java@39
PS2, Line 39: Preconditions.checkState(SERVICE_TYPE == null ||
SERVICE_TYPE.equals(serviceType),
: String.format("%s != %s", SERVICE_TYPE, serviceType));
: Preconditions.checkState(APP_ID == null ||
APP_ID.equals(appId),
: String.format("%s != %s", APP_ID, appId));
nit: This could be skipped if there are two getInstance() calls in parallel
with different configs and INSTANCE is still null. Moving the tests to the end
seem more logical to me.
--
To view, visit http://gerrit.cloudera.org:8080/15235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91f2bad1a9ce897b45cfc42f97b192fe2f8c7e06
Gerrit-Change-Number: 15235
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Tue, 18 Feb 2020 10:08:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5092 Add support for VARCHAR in Kudu tables
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/14197 ) Change subject: IMPALA-5092 Add support for VARCHAR in Kudu tables .. Patch Set 10: (2 comments) Thank you for the change. I did a first pass and found two small nits. Will do a quick test locally as well later today/tomorrow. http://gerrit.cloudera.org:8080/#/c/14197/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14197/10//COMMIT_MSG@11 PS10, Line 11: Usually, Impala commit messages also have a section "Testing" to mention the tests added by this commit. This will help reviewers understand the testing done for the patch. See this for example. https://gerrit.cloudera.org/#/c/15157/ http://gerrit.cloudera.org:8080/#/c/14197/10/be/src/exec/kudu-util.cc File be/src/exec/kudu-util.cc: http://gerrit.cloudera.org:8080/#/c/14197/10/be/src/exec/kudu-util.cc@139 PS10, Line 139: nit: Indentation should be 4 in these cases. -- To view, visit http://gerrit.cloudera.org:8080/14197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d4959410fdd882bfa980cb55e8a7837c7823da8 Gerrit-Change-Number: 14197 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 18 Feb 2020 09:38:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Backend support for Z-ordering
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 ) Change subject: IMPALA-8755: Backend support for Z-ordering .. Patch Set 26: The verification failed due to a the flaky test_exchange_mem_usage_scaling and AuthorizationStmtTest.testSelect. Run an exhaustive test, it passed: https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/6472/ -- To view, visit http://gerrit.cloudera.org:8080/14080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0200748ce3e65ebc5d3530f794c0f80aa335a2ab Gerrit-Change-Number: 14080 Gerrit-PatchSet: 26 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Anonymous Coward (520) Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 18 Feb 2020 08:55:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9392 Change Boost version for aarch64 supporting
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15231 ) Change subject: IMPALA-9392 Change Boost version for aarch64 supporting .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5354/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/15231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12f1d8fa1f3e35a62f3f42b3a2d19b85ca8c7a3d Gerrit-Change-Number: 15231 Gerrit-PatchSet: 6 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 18 Feb 2020 08:41:21 + Gerrit-HasComments: No
