[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. IMPALA-8944: Update and re-enable S3PlannerTest Addresses several test infra issues that were preventing the S3PlannerTest from running successfully. Disables a few tests that are no longer working, and removes some planner checks that are no longer applicable when running on S3. Specifically, this patch removes the checks in PlannerTestBase#checkScanRangeLocations when running against S3, because the planner no longer generates scan ranges; generation is deferred to the scheduler (IMPALA-5931). Replaces the old logic of specifying S3-specific fe/ tests with a combination of JUnit Categories and Maven Profiles. The previous method was broken and assumed that all S3-specific fe/ tests started with S3*. The new approach removes that restriction and only requires S3-specific JUnit tests to be tagged with the Java annotation '@Category(S3Tests.class)' (entire classes or individual tests can be tagged with the annotation). Testing: * Ran fe/ tests with TARGET_FILESYSTEM=s3 Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Reviewed-on: http://gerrit.cloudera.org:8080/14248 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/run-all-tests.sh M fe/pom.xml M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java A fe/src/test/java/org/apache/impala/planner/S3Tests.java 6 files changed, 79 insertions(+), 13 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 04:46:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14214 ) Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4961/ -- To view, visit http://gerrit.cloudera.org:8080/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 02:45:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 19 Sep 2019 01:49:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. IMPALA-8761: Improve events processor configuration validation. This patch aims to improve the validation of configuration keys from the HMS. IMPALA-8559 introduced configuration validation for events processor configurations. In the existing implementation, the events processor errors out as soon as it sees a validation failure. If there are more than one configuration errors, the users may have to restart HMS each time they fix a configuration error. This is bad user experience. This change collects all the configuration issues and logs expected values before erroring out. Users can now fix all issues in one go. Testing: Added testValidateConfigs() to assert if multiple incorrect values are detected at once. Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Reviewed-on: http://gerrit.cloudera.org:8080/14240 Reviewed-by: Bharath Vissapragada Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 2 files changed, 43 insertions(+), 5 deletions(-) Approvals: Bharath Vissapragada: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4595/ : 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/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 01:16:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 ) Change subject: IMPALA-7637: Add more hash table stats to profile .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@645 PS3, Line 645: CreateAndAddToProfile > nit: AddHashTableCounters I'd suggest returning a unique_ptr, and using an unique_ptr to store it - that documents the memory ownership unambiguously and will avoid any risk of memory leaks being introduced later http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@651 PS3, Line 651: StatsCountersAdd > could all the counters just be updated in the Close method instead? I think this kinda makes sense - we know the number of buckets earlier than we know the final values of the other stats. It'd be good to mention in the method comments. -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Thu, 19 Sep 2019 00:41:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4962/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 00:36:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 5: Code-Review+2 Carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 00:36:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 00:36:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Hello Bharath Vissapragada, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14248 to look at the new patch set (#5). Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. IMPALA-8944: Update and re-enable S3PlannerTest Addresses several test infra issues that were preventing the S3PlannerTest from running successfully. Disables a few tests that are no longer working, and removes some planner checks that are no longer applicable when running on S3. Specifically, this patch removes the checks in PlannerTestBase#checkScanRangeLocations when running against S3, because the planner no longer generates scan ranges; generation is deferred to the scheduler (IMPALA-5931). Replaces the old logic of specifying S3-specific fe/ tests with a combination of JUnit Categories and Maven Profiles. The previous method was broken and assumed that all S3-specific fe/ tests started with S3*. The new approach removes that restriction and only requires S3-specific JUnit tests to be tagged with the Java annotation '@Category(S3Tests.class)' (entire classes or individual tests can be tagged with the annotation). Testing: * Ran fe/ tests with TARGET_FILESYSTEM=s3 Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 --- M bin/run-all-tests.sh M fe/pom.xml M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java A fe/src/test/java/org/apache/impala/planner/S3Tests.java 6 files changed, 79 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/14248/5 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 18: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4959/ -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 18 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 19 Sep 2019 00:22:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4958/ -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Thu, 19 Sep 2019 00:13:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13329 )
Change subject: IMPALA-6433: Part 1: Extract page reading logic from
ParquetColumnReader
..
Patch Set 19:
(11 comments)
Thanks for doing all this cleanup! This is a big improvement. I had some
relatively minor suggestions.
I'd suggest doing a pass over the patch and reducing the amount of vertical
whitespace. I pointed out a couple of functions where it was out of line with
the rest of the code, but I think generally there's other opportunities to fit
more code on the screen without it becomign less readable.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc
File be/src/exec/parquet/parquet-column-readers.cc:
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc@1066
PS19, Line 1066: page_reader_.Close(row_batch->tuple_data_pool());
nit: maybe condense to:
page_reader_.Close(row_batch == nullptr ? nullptr :
row_batch->tuple_data_pool());
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc@1074
PS19, Line 1074: Status BaseScalarColumnReader::InitDictionary() {
nit: I think there's a bit too much vertical whitespace in this function now -
it doesn't really aid readability. Some parts are basically double-spaced now.
I'd suggest removing all the blank lines from l1079-1097, l1099-1112 and the
blank line before the final return.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc@1084
PS19, Line 1084: int data_size;
We should switch to int64_t for the byte value while we're here, it's just
generally safer to use in interfaces for memory values.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc@1133
PS19, Line 1133: Status BaseScalarColumnReader::ReadDataPage() {
Same comment about vertical whitespace.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-column-readers.cc@1404
PS19, Line 1404: // TODO for 2.3: node_.element->name isn't necessarily useful
we can remove the 2.3 reference here :)
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.h
File be/src/exec/parquet/parquet-page-reader-low.h:
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.h@27
PS19, Line 27: ParquetPageReaderLow
I think the class name could be improved. Maybe something like
ParquetPageReaderHelper?
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.h@93
PS19, Line 93: ///Uninitialized
Thank you for taking the time to add and document this, I like it.
Maybe it's worth labeling the state transitions with the method names, if you
can do it without cluttering it too much.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.cc
File be/src/exec/parquet/parquet-page-reader-low.cc:
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.cc@79
PS19, Line 79: // TODO: this will need to change when we have co-located
files and the columns
Can you remove this TODO, I don't think we're going to do what this is
describing.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader-low.cc@131
PS19, Line 131: *eos = false;
nit: there's a bit too much vertical whitespace here, it's become double-spaced
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader.cc
File be/src/exec/parquet/parquet-page-reader.cc:
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader.cc@97
PS19, Line 97:
nit: don't need the blank lines in this function, would be perfectly readable
and denser without them.
http://gerrit.cloudera.org:8080/#/c/13329/19/be/src/exec/parquet/parquet-page-reader.cc@252
PS19, Line 252: // TODO: can't we call
stream_->ReleaseCompletedResources(false); at this point?
Quite possibly, this code has undergone a lot of iteration so stuff like this
may have been missed.
--
To view, visit http://gerrit.cloudera.org:8080/13329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0289394adcb97a3529313030930c9c5b85aaa12
Gerrit-Change-Number: 13329
Gerrit-PatchSet: 19
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Wed, 18 Sep 2019 23:36:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4594/ : 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/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 23:32:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 5: Code-Review+2 Carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:50:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14246 to look at the new patch set (#5). Change subject: IMPALA-8634: Catalog client should retry RPCs .. IMPALA-8634: Catalog client should retry RPCs Add retries to catalogd RPCs. Previously, connection failures triggered a retry, but failures on the actual RPC did not trigger a retry. This change replaces all usages of ClientCache::DoRpc() in the CatalogOpExecutor with ClientCache::DoRpcWithRetry(). This change moves the connection retry loop to DoRpcWithRetry(), instead of relying on the ClientCache to retry the connection. This patch is based to IMPALA-8904, which adds similar functionality to statestore RPCs. Testing: * Renamed test_statestore_rpc_errors.py to test_services_rpc_errors.py and added new tests for catalogd RPC errors * Added new tests to test_restart_services.py * Ran core tests Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c --- M be/src/exec/catalog-op-executor.cc M be/src/runtime/exec-env.cc M tests/custom_cluster/test_restart_services.py A tests/custom_cluster/test_services_rpc_errors.py D tests/custom_cluster/test_statestore_rpc_errors.py 5 files changed, 232 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/14246/5 -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14246/4/tests/custom_cluster/test_services_rpc_errors.py File tests/custom_cluster/test_services_rpc_errors.py: http://gerrit.cloudera.org:8080/#/c/14246/4/tests/custom_cluster/test_services_rpc_errors.py@80 PS4, Line 80: Validte > typo Done -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:50:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8942: Set file format specific split sizes on non-block stores
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14247 ) Change subject: IMPALA-8942: Set file format specific split sizes on non-block stores .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/14247/1/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/14247/1/be/src/service/query-options.h@192 PS1, Line 192: QUERY_OPT_FN(parquet_object_store_split_size, PARQUET_OBJECT_STORE_SPLIT_ > Since this is primarily used by object stores (I think the only exception i Done http://gerrit.cloudera.org:8080/#/c/14247/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/14247/2/be/src/service/query-options.cc@884 PS2, Line 884: query_options->__set_parquet_object_store_split_size(parquet_object_store_split_size); > line too long (94 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/14247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 Gerrit-Change-Number: 14247 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 22:48:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8942: Set file format specific split sizes on non-block stores
Hello Alex Rodoni, David Rorke, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14247 to look at the new patch set (#3). Change subject: IMPALA-8942: Set file format specific split sizes on non-block stores .. IMPALA-8942: Set file format specific split sizes on non-block stores On non-block based stores (e.g. S3, ADLS, etc.), the planner creates split sizes based on the value of FileSystem.getDefaultBlockSize(Path). This does not work well for Parquet, because the scanners will only process a split if the data range defined by the split overlaps with the midpoint of the Parquet row group. This is done to ensure that scanners treat Parquet row groups as the unit of processing. The default block size for non-block based stores is typically much lower than the Parquet row group size. This causes a lot of dummy Parquet splits to be created and processed, most of which end up doing nothing. The major issue this causes is skew, and each scanner ends up processing a skewed amount of data (see IMPALA-3453 for details on the skew issue). This patch adds a new query option PARQUET_OBJECT_STORE_SPLIT_SIZE (defaults to 256 MB) that controls the size of Parquet splits on non-block stores. Impala docs actually recommend setting fs.s3a.block.size to 128 MB (row group size used by Hive / Spark) or 256 MB (row group size used by Impala). Setting the block size to the row group size results in ideal split assignment, but experiments show that using a 256 MB block size for 128 MB row groups is better than using a 128 MB block size for 256 MB row groups, so the default value of PARQUET_OBJECT_STORE_SPLIT_SIZE is 256 MB. Updated the docs accordingly. Testing: * Ran core tests * Added tests to test_scanners.py Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M docs/topics/impala_s3.xml M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M tests/query_test/test_scanners.py 7 files changed, 69 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/14247/3 -- To view, visit http://gerrit.cloudera.org:8080/14247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 Gerrit-Change-Number: 14247 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8942: Set file format specific split sizes on non-block stores
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14247 ) Change subject: IMPALA-8942: Set file format specific split sizes on non-block stores .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/14247/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/14247/2/be/src/service/query-options.cc@884 PS2, Line 884: query_options->__set_parquet_object_store_split_size(parquet_object_store_split_size); line too long (94 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 Gerrit-Change-Number: 14247 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Sep 2019 22:45:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8942: Set file format specific split sizes on non-block stores
Hello Alex Rodoni, David Rorke, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14247 to look at the new patch set (#2). Change subject: IMPALA-8942: Set file format specific split sizes on non-block stores .. IMPALA-8942: Set file format specific split sizes on non-block stores On non-block based stores (e.g. S3, ADLS, etc.), the planner creates split sizes based on the value of FileSystem.getDefaultBlockSize(Path). This does not work well for Parquet, because the scanners will only process a split if the data range defined by the split overlaps with the midpoint of the Parquet row group. This is done to ensure that scanners treat Parquet row groups as the unit of processing. The default block size for non-block based stores is typically much lower than the Parquet row group size. This causes a lot of dummy Parquet splits to be created and processed, most of which end up doing nothing. The major issue this causes is skew, and each scanner ends up processing a skewed amount of data (see IMPALA-3453 for details on the skew issue). This patch adds a new query option PARQUET_OBJECT_STORE_SPLIT_SIZE (defaults to 256 MB) that controls the size of Parquet splits on non-block stores. Impala docs actually recommend setting fs.s3a.block.size to 128 MB (row group size used by Hive / Spark) or 256 MB (row group size used by Impala). Setting the block size to the row group size results in ideal split assignment, but experiments show that using a 256 MB block size for 128 MB row groups is better than using a 128 MB block size for 256 MB row groups, so the default value of PARQUET_OBJECT_STORE_SPLIT_SIZE is 256 MB. Updated the docs accordingly. Testing: * Ran core tests * Added tests to test_scanners.py Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 --- M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M docs/topics/impala_s3.xml M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M tests/query_test/test_scanners.py 7 files changed, 68 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/14247/2 -- To view, visit http://gerrit.cloudera.org:8080/14247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 Gerrit-Change-Number: 14247 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14246/4/tests/custom_cluster/test_services_rpc_errors.py File tests/custom_cluster/test_services_rpc_errors.py: http://gerrit.cloudera.org:8080/#/c/14246/4/tests/custom_cluster/test_services_rpc_errors.py@80 PS4, Line 80: Validte typo -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:35:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@144 PS1, Line 144: 0 > Sure, this patch or a separate one? Not sure what an appropriate default fo A separate one makes sense. Please also see https://issues.apache.org/jira/browse/KUDU-2192 -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:33:59 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/6521 ) Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so .. Patch Set 10: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/6521/10/source/kudu/build.sh File source/kudu/build.sh: http://gerrit.cloudera.org:8080/#/c/6521/10/source/kudu/build.sh@98 PS10, Line 98: LD_LIBRARY_PATH="" wrap ./build-if-necessary.sh Found a minor issue building on my system where ninja was installed. The docker build was not affected. -- To view, visit http://gerrit.cloudera.org:8080/6521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 Gerrit-Change-Number: 6521 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:30:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14214 ) Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4961/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 22:30:41 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/6521 ) Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so .. Patch Set 10: I'm going to do a full docker build with the final version of the code change, then verify, then submit. -- To view, visit http://gerrit.cloudera.org:8080/6521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 Gerrit-Change-Number: 6521 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 22:31:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14214 ) Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 22:30:40 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/6521 )
Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
..
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6521/9/functions.sh
File functions.sh:
http://gerrit.cloudera.org:8080/#/c/6521/9/functions.sh@615
PS9, Line 615: print os.path.relpath('${src}', '${dst}')
> Something I forgot before: We could future-proof this for Python3 by using
Done
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh
File init-compiler.sh:
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh@62
PS9, Line 62: linker
> Nit: I think it would be clearer if we used "dynamic linker"
Done
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh@74
PS9, Line 74: We need to make sure they link against the correct libraries
because
: # the libraries are not symlinked until after the component
build completes.
> Let me check my understanding:
Done
--
To view, visit http://gerrit.cloudera.org:8080/6521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67
Gerrit-Change-Number: 6521
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Hector Acosta
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 18 Sep 2019 22:30:14 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Hello Tianyi Wang, Hector Acosta, Philip Zeyliger, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6521 to look at the new patch set (#10). Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so .. IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so C++ packages depend on these shared objects. Our toolchain packages should always be run using the toolchain version of the shared objects that it ships. Previously the toolchain artifacts were often (but not always) linked against system version of libgcc.so and libstdc++.so, which can cause compatibility problems. This is a major problem on recent Linux distros like Ubuntu 16.04 that default to the new C++11 ABI because the system libstdc++.so is not compatible with gcc-4.9.2-generated binaries. It also means behaviour of toolchain artifacts may be less consistent across systems. This patch does two things to ensure that toolchain artifacts link to the correct libraries: 1. fixes the RPATHs added to executables and shared objects so that they always point to the ../lib/ and ../lib64/ directories. 2. adds symlinks from the lib/ directory to libstd++.so and libgcc.so where required by executables and shared objects. Also remove redundant ccache enablement in the cmake build scripts. We already use ccache globally via a wrapper script. Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 --- M functions.sh M init-compiler.sh M source/cmake/build.sh M source/kudu/build.sh 4 files changed, 117 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/21/6521/10 -- To view, visit http://gerrit.cloudera.org:8080/6521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 Gerrit-Change-Number: 6521 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4593/ : 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/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 22 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Sep 2019 22:27:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14214 ) Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 22:27:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4592/ : 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/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 18 Sep 2019 22:08:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 ) Change subject: IMPALA-7637: Add more hash table stats to profile .. Patch Set 3: (9 comments) http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@532 PS3, Line 532: HashTableStatsProfile what exactly is this struct suppose to encapsulate? would be good to add some docs explaining what this struct is for. it looks like this tracks the stats of all hash tables created by a node, so it would be good to document the scope and lifecycle of this object. http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@534 PS3, Line 534: RuntimeProfile *hashtable_profile = nullptr; nit: RuntimeProfile* hasbtable_profile = nullptr; http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@643 PS3, Line 643: // Create profile and counters for HashTable stats and add to parent profile. : /// Returns a HashTableStatsProfile object. would be good to document that this actually makes a child profile called "Hash Table" http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@645 PS3, Line 645: CreateAndAddToProfile nit: AddHashTableCounters http://gerrit.cloudera.org:8080/#/c/14234/3/be/src/exec/hash-table.h@651 PS3, Line 651: StatsCountersAdd could all the counters just be updated in the Close method instead? http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py@676 PS3, Line 676: '(?<=Probes: )\d+(\.\d+)?' is the positive lookahead in this regex necessary? would the following regex not achieve the same thing: Probes:.*(\.\d+) http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py@677 PS3, Line 677: a nit: an http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py@688 PS3, Line 688: execute_query should assert that the query was actually successful: result = self.execute_query(query) assert result.success http://gerrit.cloudera.org:8080/#/c/14234/3/tests/query_test/test_observability.py@691 PS3, Line 691: nit: extra space -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Wed, 18 Sep 2019 22:00:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Fang-Yu Rao has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. IMPALA-7984: Port runtime filter from Thrift RPC to KRPC Previously the aggregation and propagation of a runtime filter in Impala is implemented using Thrift RPC, which suffers from a disadvantage that the number of connections in a cluster grows with both the number of queries and cluster size. This patch ports the functions that implement the aggregation and propagation of a runtime filter, i.e., UpdateFilter() and PublishFilter(), respctively, to KRPC, which requires only one connection per direction between every pair of hosts, thus reducing the number of connections in a cluster. In addition, this patch also incorporates KRPC sidecar when the runtime filter is a Bloom filter. KRPC sidecar eliminates the need for an extra copy of the Bloom filter contents when a Bloom filter is serialized to be transmitted and hence reduces the serialization overhead. Due to the incorporation of KRPC sidecar, a SpinLock is also added to prevent a BloomFilter from being deallocated before its associated KRPC call finishes. Two related BE tests bloom-filter-test.cc and bloom-filter-benchmark.cc are also modified accordingly because of the changes to the signatures of some functions in BloomFilter. Testing: This patch has passed the exhaustive tests. Change-Id: I6b394796d250286510e157ae326882bfc01d387a --- M be/src/benchmarks/bloom-filter-benchmark.cc M be/src/runtime/backend-client.h M be/src/runtime/client-cache.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/data-stream-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h M be/src/runtime/exec-env.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.h M be/src/runtime/timestamp-value.h M be/src/scheduling/request-pool-service.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/frontend.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/bloom-filter-test.cc M be/src/util/bloom-filter.cc M be/src/util/bloom-filter.h M be/src/util/min-max-filter-test.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/protobuf/common.proto M common/protobuf/data_stream_service.proto M common/thrift/ImpalaInternalService.thrift 39 files changed, 1,072 insertions(+), 745 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/13882/22 -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 22 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4960/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 18 Sep 2019 21:36:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 18 Sep 2019 21:36:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/14240 ) Change subject: IMPALA-8761: Improve events processor configuration validation. .. IMPALA-8761: Improve events processor configuration validation. This patch aims to improve the validation of configuration keys from the HMS. IMPALA-8559 introduced configuration validation for events processor configurations. In the existing implementation, the events processor errors out as soon as it sees a validation failure. If there are more than one configuration errors, the users may have to restart HMS each time they fix a configuration error. This is bad user experience. This change collects all the configuration issues and logs expected values before erroring out. Users can now fix all issues in one go. Testing: Added testValidateConfigs() to assert if multiple incorrect values are detected at once. Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 2 files changed, 43 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/14240/3 -- To view, visit http://gerrit.cloudera.org:8080/14240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42 Gerrit-Change-Number: 14240 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8942: Set file format specific split sizes on non-block stores
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14247 ) Change subject: IMPALA-8942: Set file format specific split sizes on non-block stores .. Patch Set 1: (1 comment) First pass, the code looks good. A quick thought on the name of the query option. I need to think through the test in test_scanners.py. http://gerrit.cloudera.org:8080/#/c/14247/1/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/14247/1/be/src/service/query-options.h@192 PS1, Line 192: QUERY_OPT_FN(parquet_non_block_split_size, PARQUET_NON_BLOCK_SPLIT_SIZE,\ Since this is primarily used by object stores (I think the only exception is the local filesystem), an alternative is to name this PARQUET_OBJECT_STORE_SPLIT_SIZE. -- To view, visit http://gerrit.cloudera.org:8080/14247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0995b2a3b732d39d6f58e9b3bb04111ac04601e6 Gerrit-Change-Number: 14247 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Sep 2019 21:02:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 17: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4591/ : 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/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 17 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 20:50:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4590/ : 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/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 20:39:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4589/ : 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/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 20:39:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4588/ : 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/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 20:32:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 18: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4959/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 18 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 20:10:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 18: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 18 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 20:10:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 17: Code-Review+2 Carrying over Andrew's +2 -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 17 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 20:09:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14103 to look at the new patch set (#17). Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. IMPALA-8858: Add metrics tracking num queries running on executor groups With this patch, all executor groups with at least one executor will have a metric added that displays the number of queries (admitted by the local coordinator) running on them. The metric is removed only when the group has no executors in it. It gets updated when either the cluster membership changes or a query gets admitted or released by the admission controller. Also adds the ability to delete metrics from a metric group after registration. Testing: Added a custom cluster test and a BE metric test. Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 --- M be/src/runtime/exec-env.cc M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/cluster-membership-mgr.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/metrics-test.cc M be/src/util/metrics.cc M be/src/util/metrics.h M common/thrift/metrics.json M tests/custom_cluster/test_executor_groups.py 13 files changed, 367 insertions(+), 212 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/14103/17 -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 17 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 17: (2 comments) http://gerrit.cloudera.org:8080/#/c/14103/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14103/16//COMMIT_MSG@10 PS16, Line 10: will have a metric added that displays the number of queries > Nit: displays Done http://gerrit.cloudera.org:8080/#/c/14103/16/tests/custom_cluster/test_executor_groups.py File tests/custom_cluster/test_executor_groups.py: http://gerrit.cloudera.org:8080/#/c/14103/16/tests/custom_cluster/test_executor_groups.py@354 PS16, Line 354: # Create an exec group of min size 2 to exercise the case where a group becomes > Nit: Create an exec group... Done -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 17 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 20:09:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 19:59:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4958/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 19:59:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 3: Code-Review+2 Carrying +2. -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 19:59:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Hello Bharath Vissapragada, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14248 to look at the new patch set (#3). Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. IMPALA-8944: Update and re-enable S3PlannerTest Addresses several test infra issues that were preventing the S3PlannerTest from running successfully. Disables a few tests that are no longer working, and removes some planner checks that are no longer applicable when running on S3. Specifically, this patch removes the checks in PlannerTestBase#checkScanRangeLocations when running against S3, because the planner no longer generates scan ranges; generation is deferred to the scheduler (IMPALA-5931). Replaces the old logic of specifying S3-specific fe/ tests with a combination of JUnit Categories and Maven Profiles. The previous method was broken and assumed that all S3-specific fe/ tests started with S3*. The new approach removes that restriction and only requires S3-specific JUnit tests to be tagged with the Java annotation '@Category(S3Tests.class)' (entire classes or individual tests can be tagged with the annotation). Testing: * Ran fe/ tests with TARGET_FILESYSTEM=s3 Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 --- M bin/run-all-tests.sh M fe/pom.xml M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java A fe/src/test/java/org/apache/impala/planner/S3Tests.java 6 files changed, 78 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/14248/3 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Hello Bharath Vissapragada, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14248 to look at the new patch set (#2). Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. IMPALA-8944: Update and re-enable S3PlannerTest Addresses several test infra issues that were preventing the S3PlannerTest from running successfully. Disables a few tests that are no longer working, and removes some planner checks that are no longer applicable when running on S3. Specifically, this patch removes the checks in PlannerTestBase#checkScanRangeLocations when running against S3, because the planner no longer generates scan ranges; generation is deferred to the scheduler (IMPALA-5931). Replaces the old logic of specifying S3-specific fe/ tests with a combination of JUnit Categories and Maven Profiles. The previous method was broken and assumed that all S3-specific fe/ tests started with S3*. The new approach removes that restriction and only requires S3-specific JUnit tests to be tagged with the Java annotation '@Category(S3Tests.class)' (entire classes or individual tests can be tagged with the annotation). Testing: * Ran fe/ tests with TARGET_FILESYSTEM=s3 Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 --- M bin/run-all-tests.sh M fe/pom.xml M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M fe/src/test/java/org/apache/impala/planner/S3PlannerTest.java A fe/src/test/java/org/apache/impala/planner/S3Tests.java 6 files changed, 78 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/14248/2 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14248/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/14248/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@57 PS1, Line 57: HBaseTestDataRegionAssignment assignment = new HBaseTestDataRegionAssignment(); > nit: Add a quick commit why this is needed? Kinda not obvious. Done -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 19:58:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@142 PS1, Line 142: DEFINE_int32(catalog_client_connection_num_retries, 3, "The number of times connections " > Seems reasonable to me. Done, interestingly 10 retries and a 3 second interval is what the statestore client is already using. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@144 PS1, Line 144: 0 > We may want to consider setting this timeout to non-zero at some point. Whe Sure, this patch or a separate one? Not sure what an appropriate default for this would be. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@180 PS1, Line 180: 1, 0 > May help to name these constants as variables. I added some code comments which I think makes it clearer than introducing variables like `catalog_client_connection_num_retries` and `catalog_client_rpc_retry_interval_ms`. It still tries to recreate new connections, but the retry is driven by the DoRpcWithRetry() loop rather than the loop in ThriftClientImpl::OpenWithRetry. The loop in DoRpcWithRetry() first tries to create a new ClientConnection, that will trigger a call to ClientCacheHelper::GetClient, which will either try to create a new connection or used a cached one. If it tries to get a new connection and the catalogd is down, the connection will fail, but the next iteration of the loop in DoRpcWithRetry() will retry to establish the connection. If GetClient finds a cached connection, the RPC will fail, and again the loop will trigger a retry. -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 19:53:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14246 to look at the new patch set (#4). Change subject: IMPALA-8634: Catalog client should retry RPCs .. IMPALA-8634: Catalog client should retry RPCs Add retries to catalogd RPCs. Previously, connection failures triggered a retry, but failures on the actual RPC did not trigger a retry. This change replaces all usages of ClientCache::DoRpc() in the CatalogOpExecutor with ClientCache::DoRpcWithRetry(). This change moves the connection retry loop to DoRpcWithRetry(), instead of relying on the ClientCache to retry the connection. This patch is based to IMPALA-8904, which adds similar functionality to statestore RPCs. Testing: * Renamed test_statestore_rpc_errors.py to test_services_rpc_errors.py and added new tests for catalogd RPC errors * Added new tests to test_restart_services.py * Ran core tests Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c --- M be/src/exec/catalog-op-executor.cc M be/src/runtime/exec-env.cc M tests/custom_cluster/test_restart_services.py A tests/custom_cluster/test_services_rpc_errors.py D tests/custom_cluster/test_statestore_rpc_errors.py 5 files changed, 232 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/14246/4 -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13329 ) Change subject: IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader .. Patch Set 19: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4957/ -- To view, visit http://gerrit.cloudera.org:8080/13329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0289394adcb97a3529313030930c9c5b85aaa12 Gerrit-Change-Number: 13329 Gerrit-PatchSet: 19 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 19:25:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. Patch Set 20: Code-Review+1 (4 comments) I'll reach out to Michael and see if he wants to do a pass, otherwise I think that this can be +2ed once you address my last few comments. Thanks again for all the work on this! http://gerrit.cloudera.org:8080/#/c/13882/20/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/13882/20/be/src/runtime/coordinator.cc@1113 PS20, Line 1113: DCHECK(!params->bloom_filter().always_false()); not really necessary http://gerrit.cloudera.org:8080/#/c/13882/20/be/src/runtime/coordinator.cc@1126 PS20, Line 1126: if I think you'll want to make this an 'else if' with the 'if' above http://gerrit.cloudera.org:8080/#/c/13882/20/be/src/runtime/runtime-filter-bank.cc File be/src/runtime/runtime-filter-bank.cc: http://gerrit.cloudera.org:8080/#/c/13882/20/be/src/runtime/runtime-filter-bank.cc@244 PS20, Line 244: Status status = bloom_filter->Init( There's a bug here - if we fail to get the sidecar above then bloom_filter will be nullptr, so we won't want to call Init() on it http://gerrit.cloudera.org:8080/#/c/13882/19/be/src/util/min-max-filter.h File be/src/util/min-max-filter.h: http://gerrit.cloudera.org:8080/#/c/13882/19/be/src/util/min-max-filter.h@22 PS19, Line 22: #include "impala-ir/impala-ir-functions.h" > Thanks for catching this! I found that we do not need 'data_stream_service' This is getting rather nit-picky of me, but just for your understanding: the goal isn't to have the smallest number of includes that will allow things to compile, the goal is to include exactly those things that are used in the file. Clearly, there are things from data_stream_service that are used in this file now, eg. MinMaxFilterPB. Presumably you don't need to include it for things to compile because its already included in on of the other headers thats included here. To be clear, I'm not saying that you need to go back through this whole patch and make sure your includes are exactly correct. If it compiles as is then that's fine. The most important things is to not include things that aren't used, since that can lead to unnecessary re-compilation in some cases. -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 20 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Sep 2019 19:22:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14214 ) Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4587/ : 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/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 18 Sep 2019 19:11:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Sep 2019 19:01:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 1: Code-Review+1 This makes sense to me. (I'm willing to bump to +2 or Bharath can do it.) -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Sep 2019 19:01:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8761: Improve events processor configuration validation.
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14240 )
Change subject: IMPALA-8761: Improve events processor configuration validation.
..
Patch Set 2: Code-Review+2
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14240/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14240/2//COMMIT_MSG@9
PS2, Line 9: This patch aims to improve the validation of configuration keys
from the HMS.
nit: line wrap
http://gerrit.cloudera.org:8080/#/c/14240/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:
http://gerrit.cloudera.org:8080/#/c/14240/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@267
PS2, Line 267: failedValidationResults
nit: validationErrors?
http://gerrit.cloudera.org:8080/#/c/14240/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@273
PS2, Line 273: if (!result.isValid()) {
nit: single line.
http://gerrit.cloudera.org:8080/#/c/14240/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@282
PS2, Line 282: failedValidationResults.size() > 0
nit: !validationErrors.isEmpty() (more concise)
--
To view, visit http://gerrit.cloudera.org:8080/14240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73480872ef93215d05c1fd922e64eb68a8a63a42
Gerrit-Change-Number: 14240
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Wed, 18 Sep 2019 18:50:25 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/6521 )
Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
..
Patch Set 9: Code-Review+2
(3 comments)
Couple nits, but otherwise, I think this looks good.
http://gerrit.cloudera.org:8080/#/c/6521/9/functions.sh
File functions.sh:
http://gerrit.cloudera.org:8080/#/c/6521/9/functions.sh@615
PS9, Line 615: print os.path.relpath('${src}', '${dst}')
Something I forgot before: We could future-proof this for Python3 by using
print with parens: print(os.path.relpath('${src}', '${dst}'))
That syntax is backwards compatible on python 2.
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh
File init-compiler.sh:
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh@62
PS9, Line 62: linker
Nit: I think it would be clearer if we used "dynamic linker"
http://gerrit.cloudera.org:8080/#/c/6521/9/init-compiler.sh@74
PS9, Line 74: We need to make sure they link against the correct libraries
because
: # the libraries are not symlinked until after the component
build completes.
Let me check my understanding:
The static linker that produced the executable knows about the gcc libs because
we pass in the gcc libraries via FULL_LPATH's -L flag. It doesn't care about
LD_LIBRARY_PATH.
The dynamic linker needs to be able to find the library at runtime. The RPATH
doesn't have it yet, because the symlink happens later. So, we need
LD_LIBRARY_PATH.
Nit: I think it would be nice for the comment to clarify that this is the
dynamic linker at runtime. Something like the dynamic linker needs to be able
to find the correct libraries at runtime etc etc.
--
To view, visit http://gerrit.cloudera.org:8080/6521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67
Gerrit-Change-Number: 6521
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Hector Acosta
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 18 Sep 2019 18:46:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8944: Update and re-enable S3PlannerTest
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14248 ) Change subject: IMPALA-8944: Update and re-enable S3PlannerTest .. Patch Set 1: Code-Review+1 (1 comment) I'll let Joe take another look. http://gerrit.cloudera.org:8080/#/c/14248/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/14248/1/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@57 PS1, Line 57: HBaseTestDataRegionAssignment assignment = new HBaseTestDataRegionAssignment(); nit: Add a quick commit why this is needed? Kinda not obvious. -- To view, visit http://gerrit.cloudera.org:8080/14248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1690b6c5346376cfd4845c72062cc237e0f9 Gerrit-Change-Number: 14248 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Wed, 18 Sep 2019 18:42:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8858: Add metrics tracking num queries running on executor groups
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/14103 ) Change subject: IMPALA-8858: Add metrics tracking num queries running on executor groups .. Patch Set 16: Code-Review+2 (2 comments) LGTM, a few typos that could be ignored http://gerrit.cloudera.org:8080/#/c/14103/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14103/16//COMMIT_MSG@10 PS16, Line 10: will have a metric added that display the number of queries Nit: displays http://gerrit.cloudera.org:8080/#/c/14103/16/tests/custom_cluster/test_executor_groups.py File tests/custom_cluster/test_executor_groups.py: http://gerrit.cloudera.org:8080/#/c/14103/16/tests/custom_cluster/test_executor_groups.py@354 PS16, Line 354: # Create and exec group of min size 2 to exercise the case where a group becomes Nit: Create an exec group... -- To view, visit http://gerrit.cloudera.org:8080/14103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539 Gerrit-Change-Number: 14103 Gerrit-PatchSet: 16 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 18 Sep 2019 18:35:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/14214 )
Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix
DCHECKs
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14214/2/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:
http://gerrit.cloudera.org:8080/#/c/14214/2/be/src/exec/buffered-plan-root-sink.h@147
PS2, Line 147: bool IsQueueEmpty(RuntimeState* state) const {
> It would be nice if we could add the following DCHECK:
Done
http://gerrit.cloudera.org:8080/#/c/14214/2/tests/query_test/test_result_spooling.py
File tests/query_test/test_result_spooling.py:
http://gerrit.cloudera.org:8080/#/c/14214/2/tests/query_test/test_result_spooling.py@404
PS2, Line 404: ecute_query_expect_debug_
> or assert False, "Expected Failure"
yeah, this pattern was actually copied from test_failpoints.py - so looks like
that has the same bug as well - filed IMPALA-8952 to fix this
had to do some re-factoring of this class as well, a few test failures popped
up when changed it to `assert False, "Expected Failure".
I moved the DEBUG_ACTION 5:GETNEXT:MEM_LIMIT_EXCEEDED|0:GETNEXT:DELAY and the
query select 1 from functional.alltypessmall a join functional.alltypessmall b
on a.id = b.id to a separate test case, mainly because the DEBUG_ACTION is
specific to the query (e.g. it injects a failure into the 5th node, and `select
* from functional.alltypes` does not even have 5 nodes).
--
To view, visit http://gerrit.cloudera.org:8080/14214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932
Gerrit-Change-Number: 14214
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 18 Sep 2019 18:34:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs
Hello Michael Ho, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14214 to look at the new patch set (#3). Change subject: IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs .. IMPALA-8924, IMPALA-8934: Result spooling failpoint tests, fix DCHECKs Adds several "failpoint" tests to test_result_spooling.py. These tests use debug_actions spread throughout buffered-plan-root-sink.cc to trigger failures while result spooling is running. The tests validate that all queries gracefully fail and do not cause any impalad crashes. Fixed a few bugs that came up when adding these tests, as well as the crash reported in IMPALA-8924 (which is now covered by the failpoint tests added in this patch). The first bug fixed was a DCHECK in SpillableRowBatchQueue::IsEmpty() where the method was being called after the queue had been closed. The fix is to only call IsEmpty() if IsOpen() returns true. The second bug was an issue in the cancellation path where BufferedPlanRootSink::GetNext would enter an infinite loop if the query was cancelled and then GetNext was called. The fix is to check the cancellation state in the outer while loop. Testing: * Added new tests to test_result_spooling.py * Ran core tests Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 --- M be/src/exec/buffered-plan-root-sink.cc M be/src/exec/buffered-plan-root-sink.h M tests/common/impala_test_suite.py M tests/query_test/test_result_spooling.py A tests/util/failpoints_util.py 5 files changed, 137 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/14214/3 -- To view, visit http://gerrit.cloudera.org:8080/14214 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib96f797bc8a5ba8baf9fb28abd1f645345bbe932 Gerrit-Change-Number: 14214 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-8948: [DOCS] Impala cannot write to compressed text file
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14237 ) Change subject: IMPALA-8948: [DOCS] Impala cannot write to compressed text file .. IMPALA-8948: [DOCS] Impala cannot write to compressed text file Change-Id: I7eac0431f3daeb1c3102c6a58670bce0e899d5f2 Reviewed-on: http://gerrit.cloudera.org:8080/14237 Tested-by: Impala Public Jenkins Reviewed-by: Vincent Tran Reviewed-by: Tim Armstrong --- M docs/topics/impala_file_formats.xml 1 file changed, 5 insertions(+), 6 deletions(-) Approvals: Impala Public Jenkins: Verified Vincent Tran: Looks good to me, but someone else must approve Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/14237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7eac0431f3daeb1c3102c6a58670bce0e899d5f2 Gerrit-Change-Number: 14237 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran
[Impala-ASF-CR] IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14249 ) Change subject: IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/489/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89410128acfc31d5072cf04a28bef26221f39f3 Gerrit-Change-Number: 14249 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 18:25:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8948: [DOCS] Impala cannot write to compressed text file
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14237 ) Change subject: IMPALA-8948: [DOCS] Impala cannot write to compressed text file .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7eac0431f3daeb1c3102c6a58670bce0e899d5f2 Gerrit-Change-Number: 14237 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Wed, 18 Sep 2019 18:24:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8948: [DOCS] Impala cannot write to compressed text file
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/14237 ) Change subject: IMPALA-8948: [DOCS] Impala cannot write to compressed text file .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7eac0431f3daeb1c3102c6a58670bce0e899d5f2 Gerrit-Change-Number: 14237 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Wed, 18 Sep 2019 18:11:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions
Hello Greg Rahn, Norbert Luksa, Zoltan Borok-Nagy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14249 to look at the new patch set (#3). Change subject: IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions .. IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions Change-Id: Id89410128acfc31d5072cf04a28bef26221f39f3 --- M docs/topics/impala_string_functions.xml 1 file changed, 273 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/14249/3 -- To view, visit http://gerrit.cloudera.org:8080/14249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id89410128acfc31d5072cf04a28bef26221f39f3 Gerrit-Change-Number: 14249 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14249 ) Change subject: IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/489/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89410128acfc31d5072cf04a28bef26221f39f3 Gerrit-Change-Number: 14249 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 17:59:01 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/6521 )
Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
..
Patch Set 8:
(2 comments)
I had to make a couple of other changes:
* I had some issues with ccache and cmake locally, so disabled the redundant
ccache enablement for the cmake build
* I moved the LD_LIBRARY_PATH setup to a shared location instead of doing it
per-component, since I ran into the same problem for cmake, flatbuffers and
kudu.
http://gerrit.cloudera.org:8080/#/c/6521/7/functions.sh
File functions.sh:
http://gerrit.cloudera.org:8080/#/c/6521/7/functions.sh@612
PS7, Line 612: nction calc_relpath() {
> This is stale. and must be relative paths from the curre
Done
http://gerrit.cloudera.org:8080/#/c/6521/7/functions.sh@626
PS7, Line 626: # The lib/ subfolder is on the rpath of all binaries. Add a
symlink there.
: if [[ "$(basename "$binary_dir")" == "lib" ]]; then
: # Don't insert extraneous ../lib
: local dst_lib_dir="$binary_dir"
: else
: local dst_lib_dir="$binary_dir/../lib"
: fi
: loca
> I'm thinking we might want to use python for this relative path calculation
For some reason I didn't think we could use python when I wrote this, but it is
clearly a dependency based on the dockerfiles. This is nicer.
--
To view, visit http://gerrit.cloudera.org:8080/6521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67
Gerrit-Change-Number: 6521
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Hector Acosta
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 18 Sep 2019 17:17:21 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Hello Tianyi Wang, Hector Acosta, Philip Zeyliger, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6521 to look at the new patch set (#9). Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so .. IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so C++ packages depend on these shared objects. Our toolchain packages should always be run using the toolchain version of the shared objects that it ships. Previously the toolchain artifacts were often (but not always) linked against system version of libgcc.so and libstdc++.so, which can cause compatibility problems. This is a major problem on recent Linux distros like Ubuntu 16.04 that default to the new C++11 ABI because the system libstdc++.so is not compatible with gcc-4.9.2-generated binaries. It also means behaviour of toolchain artifacts may be less consistent across systems. This patch does two things to ensure that toolchain artifacts link to the correct libraries: 1. fixes the RPATHs added to executables and shared objects so that they always point to the ../lib/ and ../lib64/ directories. 2. adds symlinks from the lib/ directory to libstd++.so and libgcc.so where required by executables and shared objects. Also remove redundant ccache enablement in the cmake build scripts. We already use ccache globally via a wrapper script. Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 --- M functions.sh M init-compiler.sh M source/cmake/build.sh 3 files changed, 111 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/21/6521/9 -- To view, visit http://gerrit.cloudera.org:8080/6521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 Gerrit-Change-Number: 6521 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so
Hello Tianyi Wang, Hector Acosta, Philip Zeyliger, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6521 to look at the new patch set (#8). Change subject: IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so .. IMPALA-3926: fix RPATH for libstdc++.so and libgcc.so C++ packages depend on these shared objects. Our toolchain packages should always be run using the toolchain version of the shared objects that it ships. Previously the toolchain artifacts were often (but not always) linked against system version of libgcc.so and libstdc++.so, which can cause compatibility problems. This is a major problem on recent Linux distros like Ubuntu 16.04 that default to the new C++11 ABI because the system libstdc++.so is not compatible with gcc-4.9.2-generated binaries. It also means behaviour of toolchain artifacts may be less consistent across systems. This patch does two things to ensure that toolchain artifacts link to the correct libraries: 1. fixes the RPATHs added to executables and shared objects so that they always point to the ../lib/ and ../lib64/ directories. 2. adds symlinks from the lib/ directory to libstd++.so and libgcc.so where required by executables and shared objects. Also remove redundant ccache enablement in the cmake build scripts. We already use ccache globally via a wrapper script. Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 --- M functions.sh M init-compiler.sh M source/cmake/build.sh 3 files changed, 112 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/21/6521/8 -- To view, visit http://gerrit.cloudera.org:8080/6521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3f8481a8dfe35273a763586e9d2da0d4008ac67 Gerrit-Change-Number: 6521 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Hector Acosta Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] [DOCS] Fixed a typo in Impala logs location to /var/log
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14254 ) Change subject: [DOCS] Fixed a typo in Impala logs location to /var/log .. [DOCS] Fixed a typo in Impala logs location to /var/log Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Reviewed-on: http://gerrit.cloudera.org:8080/14254 Reviewed-by: Alex Rodoni Tested-by: Impala Public Jenkins --- M docs/topics/impala_logging.xml 1 file changed, 19 insertions(+), 21 deletions(-) Approvals: Alex Rodoni: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Gerrit-Change-Number: 14254 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. Patch Set 20: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4586/ : 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/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 20 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 18 Sep 2019 16:49:53 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Fixed a typo in Impala logs location to /var/log
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14254 ) Change subject: [DOCS] Fixed a typo in Impala logs location to /var/log .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/488/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Gerrit-Change-Number: 14254 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 16:44:48 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Fixed a typo in Impala logs location to /var/log
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14254 ) Change subject: [DOCS] Fixed a typo in Impala logs location to /var/log .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Gerrit-Change-Number: 14254 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 16:22:40 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Fixed a typo in Impala logs location to /var/log
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14254 Change subject: [DOCS] Fixed a typo in Impala logs location to /var/log .. [DOCS] Fixed a typo in Impala logs location to /var/log Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 --- M docs/topics/impala_logging.xml 1 file changed, 19 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/14254/1 -- To view, visit http://gerrit.cloudera.org:8080/14254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Gerrit-Change-Number: 14254 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] [DOCS] Fixed a typo in Impala logs location to /var/log
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14254 ) Change subject: [DOCS] Fixed a typo in Impala logs location to /var/log .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/488/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I595d616d54b2896f2f4352ebf8568a5a3a42a366 Gerrit-Change-Number: 14254 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 16:22:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7984: Port runtime filter from Thrift RPC to KRPC
Fang-Yu Rao has uploaded a new patch set (#20). ( http://gerrit.cloudera.org:8080/13882 ) Change subject: IMPALA-7984: Port runtime filter from Thrift RPC to KRPC .. IMPALA-7984: Port runtime filter from Thrift RPC to KRPC Previously the aggregation and propagation of a runtime filter in Impala is implemented using Thrift RPC, which suffers from a disadvantage that the number of connections in a cluster grows with both the number of queries and cluster size. This patch ports the functions that implement the aggregation and propagation of a runtime filter, i.e., UpdateFilter() and PublishFilter(), respctively, to KRPC, which requires only one connection per direction between every pair of hosts, thus reducing the number of connections in a cluster. In addition, this patch also incorporates KRPC sidecar when the runtime filter is a Bloom filter. KRPC sidecar eliminates the need for an extra copy of the Bloom filter contents when a Bloom filter is serialized to be transmitted and hence reduces the serialization overhead. Due to the incorporation of KRPC sidecar, a SpinLock is also added to prevent a BloomFilter from being deallocated before its associated KRPC call finishes. Two related BE tests bloom-filter-test.cc and bloom-filter-benchmark.cc are also modified accordingly because of the changes to the signatures of some functions in BloomFilter. Testing: This patch has passed the exhaustive tests. Change-Id: I6b394796d250286510e157ae326882bfc01d387a --- M be/src/benchmarks/bloom-filter-benchmark.cc M be/src/runtime/backend-client.h M be/src/runtime/client-cache.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator-filter-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/data-stream-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h M be/src/runtime/exec-env.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/runtime/runtime-filter.h M be/src/runtime/timestamp-value.h M be/src/scheduling/request-pool-service.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/frontend.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/util/bloom-filter-test.cc M be/src/util/bloom-filter.cc M be/src/util/bloom-filter.h M be/src/util/min-max-filter-test.cc M be/src/util/min-max-filter.cc M be/src/util/min-max-filter.h M common/protobuf/common.proto M common/protobuf/data_stream_service.proto M common/thrift/ImpalaInternalService.thrift 39 files changed, 1,066 insertions(+), 738 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/13882/20 -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 20 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13329 ) Change subject: IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader .. Patch Set 19: Code-Review+1 I'm still not convinced about the names 'ParquetPageReader' and 'ParquetPageReaderLow'. They are too similar and the suffix "Low" doesn't really tell me what it does. Other than that Lgtm. -- To view, visit http://gerrit.cloudera.org:8080/13329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0289394adcb97a3529313030930c9c5b85aaa12 Gerrit-Change-Number: 13329 Gerrit-PatchSet: 19 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 15:46:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/14249 ) Change subject: IMPALA-8861: [DOCS] Documented Jaro and Jaro-Winkler functions .. Patch Set 2: Code-Review+1 Lgtm, I can give the final +2 once Norbert reviewed it. -- To view, visit http://gerrit.cloudera.org:8080/14249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id89410128acfc31d5072cf04a28bef26221f39f3 Gerrit-Change-Number: 14249 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 15:41:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13329 ) Change subject: IMPALA-6433: Part 1: Extract page reading logic from ParquetColumnReader .. Patch Set 19: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4957/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0289394adcb97a3529313030930c9c5b85aaa12 Gerrit-Change-Number: 13329 Gerrit-PatchSet: 19 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 18 Sep 2019 15:19:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14108 ) Change subject: IMPALA-8842 part 2: (Hive3) Use 'engine' field in HMS stat API .. Patch Set 1: > (2 comments) Thanks for taking a look. Currently this patch is blocked by a Hive3 bug, hopefully it will be fixed soon and we can move this forward. -- To view, visit http://gerrit.cloudera.org:8080/14108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28e383156c48c7038e8bc235a3f3e5cc58a4fc01 Gerrit-Change-Number: 14108 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 15:12:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 22: (2 comments) Thanks for making the changes! http://gerrit.cloudera.org:8080/#/c/13722/22/be/src/runtime/date-parse-util.cc File be/src/runtime/date-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/13722/22/be/src/runtime/date-parse-util.cc@141 PS22, Line 141: string result; nit/efficiency: if you call string::append() in a loop, it is useful to call string::reserve() before the loop to avoid resizing the result string too many times: result.reserve(dt_ctx->fmt_out_len); http://gerrit.cloudera.org:8080/#/c/13722/22/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/13722/22/be/src/runtime/timestamp-parse-util.cc@223 PS22, Line 223: string result; same here -- To view, visit http://gerrit.cloudera.org:8080/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 22 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 14:25:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 ) Change subject: IMPALA-7637: Add more hash table stats to profile .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4585/ : 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/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Wed, 18 Sep 2019 13:47:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 ) Change subject: IMPALA-7637: Add more hash table stats to profile .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4584/ : 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/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Wed, 18 Sep 2019 13:27:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Hello Bharath Vissapragada, Sahil Takiar, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14234 to look at the new patch set (#3). Change subject: IMPALA-7637: Add more hash table stats to profile .. IMPALA-7637: Add more hash table stats to profile Add hash table counters(probes, travel and resizes) to profile. Put hash table stats into the child profile "hash table". Tests: Add new test test_query_profle_hashtable. Ran exhaustive tests. Profile Sample: Hash Join Builder (join_node_id=2): ... Runtime filters: 1 of 1 Runtime Filter Published - BuildRowsPartitionTime: 157.960us - BuildRowsPartitioned: 100 (100) - HashTablesBuildTime: 298.817us - LargestPartitionPercent: 7 (7) - MaxPartitionLevel: 0 (0) - NumRepartitions: 0 (0) - PartitionsCreated: 16 (16) - PeakMemoryUsage: 17.12 KB (17536) - RepartitionTime: 0.000ns - SpilledPartitions: 0 (0) Hash Table: - HashBuckets: 256 (256) - HashCollisions: 0 (0) - Probes: 2.52K (2520) - Resizes: 0 (0) - Travel: 1.79K (178 Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 --- M be/src/exec/grouping-aggregator-partition.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M tests/query_test/test_observability.py 8 files changed, 97 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/14234/3 -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 3 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/14234 )
Change subject: IMPALA-7637: Add more hash table stats to profile
..
Patch Set 2:
(3 comments)
New patch refactoring code to address the review issues, add support for group
aggregation profiling, add more check for tests and re-ran exhaustive tests
http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:
http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/hash-table.h@741
PS1, Line 741: /// Returns an iterator at the beginning of the hash table.
Advancing this iterator
> nit: we usually use lower_case for trivial accessor functions (this is allo
Agree, number of buckets follows the pattern. I removed these new methods for
they are no longer needed.
http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/partitioned-hash-join-builder.cc
File be/src/exec/partitioned-hash-join-builder.cc:
http://gerrit.cloudera.org:8080/#/c/14234/1/be/src/exec/partitioned-hash-join-builder.cc@595
PS1, Line 595: }
> Found the place in grouping-aggregator, I will put the similar code in a co
Done
http://gerrit.cloudera.org:8080/#/c/14234/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:
http://gerrit.cloudera.org:8080/#/c/14234/1/tests/query_test/test_observability.py@675
PS1, Line 675: assert "Resizes:" in runtime_profile
: nprobes = re.search('(?<=Probes: )\d+(\.\d+)?',
runtime_profile)
: # Probes and travel can be 0. The number can be a integer or
float with K
: assert float(nprobes.group(0)) >= 0
: ntravel = re.search('(?<=Travel: )\d
> That's a good point. I think it would be easy with the way the code is stru
Add assert on values, I just found , at least in aggregation, every counter
value can be 0.
Also, add the test for group by queries.
--
To view, visit http://gerrit.cloudera.org:8080/14234
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87
Gerrit-Change-Number: 14234
Gerrit-PatchSet: 2
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Comment-Date: Wed, 18 Sep 2019 13:05:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14234 ) Change subject: IMPALA-7637: Add more hash table stats to profile .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/14234/2/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: http://gerrit.cloudera.org:8080/#/c/14234/2/be/src/exec/hash-table.cc@425 PS2, Line 425: statsProfile->num_hash_collisions_ = ADD_COUNTER(hashtable_profile, "HashCollisions", TUnit::UNIT); line too long (101 > 90) http://gerrit.cloudera.org:8080/#/c/14234/2/be/src/exec/hash-table.cc@426 PS2, Line 426: statsProfile->num_hash_buckets_ = ADD_COUNTER(hashtable_profile, "HashBuckets", TUnit::UNIT); line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/14234/2/be/src/exec/hash-table.cc@427 PS2, Line 427: statsProfile->num_hash_resizes_ = ADD_COUNTER(hashtable_profile, "Resizes", TUnit::UNIT); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/14234/2/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/14234/2/tests/query_test/test_observability.py@677 PS2, Line 677: flake8: W291 trailing whitespace http://gerrit.cloudera.org:8080/#/c/14234/2/tests/query_test/test_observability.py@677 PS2, Line 677: # Probes and travel can be 0. The number can be a integer or float with K line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/14234/2/tests/query_test/test_observability.py@683 PS2, Line 683: " flake8: E501 line too long (91 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen Gerrit-Comment-Date: Wed, 18 Sep 2019 12:48:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7637: Add more hash table stats to profile
Hello Bharath Vissapragada, Sahil Takiar, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14234 to look at the new patch set (#2). Change subject: IMPALA-7637: Add more hash table stats to profile .. IMPALA-7637: Add more hash table stats to profile Add hash table counters(probes, travel and resizes) to profile. Put hash table stats into the child profile "hash table". Tests: Add new test test_query_profle_hashtable. Ran exhaustive tests. Profile Sample: Hash Join Builder (join_node_id=2): ... Runtime filters: 1 of 1 Runtime Filter Published - BuildRowsPartitionTime: 157.960us - BuildRowsPartitioned: 100 (100) - HashTablesBuildTime: 298.817us - LargestPartitionPercent: 7 (7) - MaxPartitionLevel: 0 (0) - NumRepartitions: 0 (0) - PartitionsCreated: 16 (16) - PeakMemoryUsage: 17.12 KB (17536) - RepartitionTime: 0.000ns - SpilledPartitions: 0 (0) Hash Table: - HashBuckets: 256 (256) - HashCollisions: 0 (0) - Probes: 2.52K (2520) - Resizes: 0 (0) - Travel: 1.79K (178 Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 --- M be/src/exec/grouping-aggregator-partition.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M tests/query_test/test_observability.py 8 files changed, 91 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/14234/2 -- To view, visit http://gerrit.cloudera.org:8080/14234 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1fd875dd1af8031242fd5f5ff554d3a71aaa6f87 Gerrit-Change-Number: 14234 Gerrit-PatchSet: 2 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongzhi Chen
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4583/ : 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/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 22 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 18 Sep 2019 09:25:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/13722 )
Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..
Patch Set 22:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/exprs/timestamp-functions-ir.cc@73
PS20, Line 73: for
> Maybe this should be called 'buff' to match the formal parameter name in th
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-parse-util.cc
File be/src/runtime/date-parse-util.cc:
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-parse-util.cc@135
PS20, Line 135: HECK(dt_ctx.
> 'buff' should either be a string* param or the return value of Format() to
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-parse-util.cc@138
PS20, Line 138: int year, month, day;
> This probably not necessary. Format should work even if string is not empty
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-parse-util.cc@180
PS20, Line 180: result.append(str_val, str_val_len);
> I'm not sure what happens if str_val == nullptr and str_val_len == 0.
it can't be nullptr, but I added a dcheck just in case.
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-parse-util.cc@180
PS20, Line 180: result.append(str_val, str_val_len);
> No need to create a temp string, you can append char* directly:
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-value.cc
File be/src/runtime/date-value.cc:
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/date-value.cc@86
PS20, Line 86: ) const {
> Again, 'buff' should be a string* param or the return value of Format().
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@113
PS20, Line 113: string token_to_probe(*curre
> nit: Probably not necessary, L110 and L111 implies that his is true.
Done
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:
http://gerrit.cloudera.org:8080/#/c/13722/20/be/src/runtime/timestamp-parse-util.cc@218
PS20, Line 218: string TimestampParser::Format(const DateTimeFormatContext&
dt_ctx,
: const date& d, const time_duration& t) {
: DCHECK(dt_ctx.toks.size() > 0);
: if (dt_ctx.has_date_toks && d.is_special()) return "";
: if (dt_ctx.has_time_toks && t.is_special()) return "";
: string result;
: for (const DateTimeFormatToken& tok: dt_ctx.toks) {
: int32_t num_val = -1;
: const char* str_val = NULL;
: int str_val_len = 0;
: switch (tok.type) {
: case YEAR:
: case ROUND_YEAR: {
: num_val = d.year();
: if (tok.len < 4) {
: int adjust_factor = std::pow(10, tok.len);
: num_val %= adjust_factor;
: }
: break;
: }
: case MONTH_IN_YEAR: num_val = d.month().as_number();
break;
: case MONTH_IN_YEAR_SLT: {
: str_val = d.month().as_short_string();
: str_val_len = 3;
: break;
: }
: case DAY_IN_MONTH: num_val = d.day(); break;
: case DAY_IN_YEAR: {
: num_val = GetDayInYear(d.year(), d.month(), d.day());
: break;
: }
: case HOUR_IN_DAY: num_val = t.hours(); break;
: case HOUR_IN_HALF_DAY: {
: num_val = t.hours();
: if (num_val == 0) num_val = 12;
: if (num_val > 12) num_val -= 12;
: break;
: }
: case MERIDIEM_INDICATOR: {
: const MERIDIEM_INDICATOR_TEXT* indicator_txt = (tok.len
== 2) ? : _LONG;
: if (t.hours() >= 12) {
: indicator_txt = (tok.len == 2) ? : _LONG;
: }
: str_val_len = tok.len;
: str_val = (isupper(*tok.val)) ? indicator_txt->first :
indicator_txt->second;
: break;
: }
: case MINUTE_IN_HOUR: num_val = t.minutes(); break;
: case SECOND_IN_MINUTE: num_val = t.seconds(); break;
: case
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13722
to look at the new patch set (#22).
Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..
IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
This enhancement introduces FORMAT clause for CAST() operator that is
applicable for casts between string types and timestamp types. Instead
of accepting SimpleDateFormat patterns the FORMAT clause supports
datetime patterns following the ISO:SQL:2016 standard.
Note, the CAST() operator without the FORMAT clause still uses
Impala's implementation of SimpleDateFormat handling. Similarly, the
existing conversion functions such as to_timestamp(), from_timestamp()
etc. remain unchanged and use SimpleDateFormat. Contrary to how these
functions work the FORMAT clause must specify a string literal and
cannot be used with any other kind of a string expression.
Milestone 1 contains all the format tokens covered by the SQL
standard. Further milestones will add more functionality on top of
this list to cover functionality provided by other RDBMS systems.
List of tokens implemented by this change:
- , YYY, YY, Y: Year tokens
- , RR: Round year tokens
- MM: Month (1-12)
- DD: Day (1-31)
- DDD: Day of year (1-366)
- HH, HH12: Hour of day (1-12)
- HH24: Hour of day (0-23)
- MI: Minute (0-59)
- SS: Second (0-59)
- S: Second of day (0-86399)
- FF, FF1, ..., FF9: Fractional second
- AM, PM, A.M., P.M.: Meridiem indicators
- TZH: Timezone hour (-99-+99)
- TZM: Timezone minute (0-99)
- Separators: - . / , ' ; : space
- ISO8601 date indicators (T, Z)
Some notes about the matching algorithm:
- The parsing algorithm uses these tokens in a case insensitive
manner.
- The separators are interchangeable with each other. For example a
'-' separator in the format will match with a '.' character in the
input.
- The length of the separator sequences is handled flexibly meaning
that a single separator character in the format for instance would
match with a multi-separator sequence in the input.
- In a string type to timestamp conversion the timezone offset tokens
are parsed, expected to match with the input but they don't adjust
the result as the input is already expected to be in UTC format.
Usage example:
SELECT CAST('01-02-2019' AS TIMESTAMP FORMAT 'MM-DD-');
SELECT CAST('2019.10.10 13:30:40.123456 +01:30' AS TIMESTAMP
FORMAT '-MM-DD HH24:MI:SS.FF9 TZH:TZM');
SELECT CAST(timestamp_column as STRING
FORMAT " MM HH12 YY") from some_table;
Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
---
M be/src/benchmarks/convert-timestamp-benchmark.cc
M be/src/benchmarks/parse-timestamp-benchmark.cc
M be/src/common/init.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/CMakeLists.txt
A be/src/exprs/cast-format-expr.cc
A be/src/exprs/cast-format-expr.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/date-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.h
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/date-parse-util.cc
M be/src/runtime/date-parse-util.h
M be/src/runtime/date-test.cc
M be/src/runtime/date-value.cc
M be/src/runtime/date-value.h
A be/src/runtime/datetime-iso-sql-format-parser.cc
A be/src/runtime/datetime-iso-sql-format-parser.h
A be/src/runtime/datetime-iso-sql-format-tokenizer.cc
A be/src/runtime/datetime-iso-sql-format-tokenizer.h
D be/src/runtime/datetime-parse-util.h
A be/src/runtime/datetime-parser-common.cc
A be/src/runtime/datetime-parser-common.h
R be/src/runtime/datetime-simple-date-format-parser.cc
A be/src/runtime/datetime-simple-date-format-parser.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.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/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/testutil/random-vector-generators.h
M be/src/util/dict-test.cc
M be/src/util/min-max-filter-test.cc
M be/src/util/string-parser.h
M common/thrift/Exprs.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A
testdata/workloads/functional-query/queries/QueryTest/cast_format_from_table.test
M testdata/workloads/functional-query/queries/QueryTest/date.test
A tests/query_test/test_cast_with_format.py
54 files changed, 3,519 insertions(+), 972 deletions(-)
git
[Impala-ASF-CR] IMPALA-8634: Catalog client should retry RPCs
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14246 ) Change subject: IMPALA-8634: Catalog client should retry RPCs .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@142 PS1, Line 142: DEFINE_int32(catalog_client_connection_num_retries, 3, "The number of times connections " > Yeah, 10 seconds does seem a little long. How about 10 retries, with an int Seems reasonable to me. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@144 PS1, Line 144: 0 We may want to consider setting this timeout to non-zero at some point. When we convert the Catalog clients to use KRPC, we may be able to rely on using TCP_USER_TIMEOUT which seems more appropriate than socket timeout. http://gerrit.cloudera.org:8080/#/c/14246/1/be/src/runtime/exec-env.cc@180 PS1, Line 180: 1, 0 May help to name these constants as variables. IIUC, this change relies on the sleep and retry in the retry loop of DoRpcWithRetry() so simply don't try recreating new connections on failure now. -- To view, visit http://gerrit.cloudera.org:8080/14246 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f33ad2b36d301fb64e70a939e71decab0ca993c Gerrit-Change-Number: 14246 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 18 Sep 2019 06:52:15 + Gerrit-HasComments: Yes
