[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3661/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 06:40:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 06:40:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1852/ : 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/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 01:54:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7800: Reject new connections after --fe service threads
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12226 ) Change subject: IMPALA-7800: Reject new connections after --fe_service_threads .. Patch Set 6: I meant "the timeout can also be turned off by setting it to 0 or something by user". We will have a non-zero timeout by default. -- To view, visit http://gerrit.cloudera.org:8080/12226 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9d48aaff9e3b5854b5121798211c641c58a559 Gerrit-Change-Number: 12226 Gerrit-PatchSet: 6 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 23 Jan 2019 01:51:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7800: Reject new connections after --fe service threads
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12226 ) Change subject: IMPALA-7800: Reject new connections after --fe_service_threads .. Patch Set 6: It seems like a more gradual approach to tackling IMPALA-7800 is to implement a timeout instead of a boolean flag which rejects the new connection right away. The behavior of instant rejection can be emulated by having a really short timeout and we can have a large default timeout. The timeout should also be disabled by setting it to 0 or something. -- To view, visit http://gerrit.cloudera.org:8080/12226 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9d48aaff9e3b5854b5121798211c641c58a559 Gerrit-Change-Number: 12226 Gerrit-PatchSet: 6 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 23 Jan 2019 01:48:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1851/ : 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/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 01:39:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 01:38:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment or query ids.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 ) Change subject: IMPALA-6664: Tag log statements with fragment or query ids. .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3660/ -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 7 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 23 Jan 2019 01:23:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4555: Make QueryState's status reporting more robust
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12049 ) Change subject: IMPALA-4555: Make QueryState's status reporting more robust .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1850/ : 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/12049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6007013fc2c9e8eeba11b752ee58fb3038da971 Gerrit-Change-Number: 12049 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 01:16:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc File be/src/util/cgroup-util.cc: http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55 PS9, Line 55: if (!proc_cgroups.good()) continue; > i think at this point it ll be an overkill to parse this, maybe just return Done -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 01:14:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12120 to look at the new patch set (#13). Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. IMPALA-7941: part 1: detect cgroups memory limit This adds the logic to detect the cgroups memory limit, but does not use it to set the process memory limit yet. The information obtained is logged at startup and accessible through the Web UI. The patch boils down to reading from the appropriate places in the filesystem. The main complication is that paths need to be translated to point to the right cgroup node inside the container. This deletes some useless cgroup logic from ProcessStateInfo that printed whatever happened to be the last cgroup in a file. Testing: Added a unit test to check that the code could parse the cgroups hierarchy ok and return a positive value. Tested on CentOS6 and CentOS7. Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 --- M be/src/common/init.cc M be/src/util/CMakeLists.txt A be/src/util/cgroup-util.cc A be/src/util/cgroup-util.h M be/src/util/default-path-handlers.cc M be/src/util/proc-info-test.cc M be/src/util/process-state-info.cc M be/src/util/process-state-info.h M www/root.tmpl 9 files changed, 289 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/13 -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12120 to look at the new patch set (#12). Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. IMPALA-7941: part 1: detect cgroups memory limit This adds the logic to detect the cgroups memory limit, but does not use it to set the process memory limit yet. The information obtained is logged at startup and accessible through the Web UI. The patch boils down to reading from the appropriate places in the filesystem. The main complication is that paths need to be translated to point to the right cgroup node inside the container. This deletes some useless cgroup logic from ProcessStateInfo that printed whatever happened to be the last cgroup in a file. Testing: Added a unit test to check that the code could parse the cgroups hierarchy ok and return a positive value. Tested on CentOS6 and CentOS7. Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 --- M be/src/common/init.cc M be/src/util/CMakeLists.txt A be/src/util/cgroup-util.cc A be/src/util/cgroup-util.h M be/src/util/default-path-handlers.cc M be/src/util/proc-info-test.cc M be/src/util/process-state-info.cc M be/src/util/process-state-info.h M www/root.tmpl 9 files changed, 285 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/12 -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5872: Testcase builder for query planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12221 ) Change subject: IMPALA-5872: Testcase builder for query planner .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1849/ : 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/12221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52 Gerrit-Change-Number: 12221 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 23 Jan 2019 00:59:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 00:54:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. IMPALA-8073: Fix FE tests that leak HMS connections This patch fixes FE tests that leak the HMS connections. I was to reproduce the issue by in SentryTestProxy by looping the test multiple times. Testing: - Ran SentryProxyTest in a loop without any issue. - Ran all FE tests Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Reviewed-on: http://gerrit.cloudera.org:8080/12241 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java 12 files changed, 206 insertions(+), 180 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc File be/src/util/cgroup-util.cc: http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55 PS9, Line 55: if (!proc_cgroups.good()) continue; > I also checked this and it doesn't look like : is escaped in the paths. Not i think at this point it ll be an overkill to parse this, maybe just return an error if fields.size() > 3. Another case that we dont seem to tackle here is when multiple subsystems are attached to the same cgroup hierarchy and the corresponding line will look like the example you just mentioned above: '9:cpu,cpuacct:/user.slice/foo:bar'. For this case we will have to split fields[1] on ',' too and look for the subsystem OR use string::find -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 Jan 2019 00:45:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8091: incremental improvements to NTP sync
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12253 ) Change subject: IMPALA-8091: incremental improvements to NTP sync .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1848/ : 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/12253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc8cacf81765d132dd574993f8149231bdbb7bc6 Gerrit-Change-Number: 12253 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Brown Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 23 Jan 2019 00:45:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5872: Testcase builder for query planner
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12221 ) Change subject: IMPALA-5872: Testcase builder for query planner .. Patch Set 2: Thanks for the comments Paul. PS2 consolidates the logic somewhat and adds a stand-alone testcase loader in Java. Still working on: - writing tests - Hooking up the in-memory catalog with a derby based HMS instead of polluting it with the postgres backing our mini-cluster HMS. I wanted to get some feedback on the whole approach while I'm working on the above stuff, hence pushing out PS2 a bit early. -- To view, visit http://gerrit.cloudera.org:8080/12221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52 Gerrit-Change-Number: 12221 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 23 Jan 2019 00:27:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4555: Make QueryState's status reporting more robust
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12049 to look at the new patch set (#3). Change subject: IMPALA-4555: Make QueryState's status reporting more robust .. IMPALA-4555: Make QueryState's status reporting more robust QueryState periodically collects runtime profiles from all of its fragment instances and sends them to the coordinator. Previously, each time this happens, if the rpc fails, QueryState will retry twice after a configurable timeout and then cancel the fragment instances under the assumption that the coordinator no longer exists. We've found in real clusters that this logic is too sensitive to failed rpcs and can result in fragment instances being cancelled even in cases where the coordinator is still running. This patch makes a few improvements to this logic: - When a report fails to send, instead of retrying the same report quickly (after waiting report_status_retry_interval_ms), we wait the regular reporting interval (status_report_interval_ms), regenerate any stale portions of the report, and then retry. - A new flag, --status_report_max_retries, is introduced, which controls the number of failed reports that are allowed before the query is cancelled. --report_status_retry_interval_ms is removed. - Exponential backoff is used, such that for a period between retries of 't', on try 'n' the actual timeout will be t * n. Testing: - Added a test which results in a large number of failed intermediate status reports but still succeeds. Change-Id: Ib6007013fc2c9e8eeba11b752ee58fb3038da971 --- M be/src/common/global-flags.cc M be/src/runtime/coordinator-backend-state.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 common/protobuf/control_service.proto M tests/custom_cluster/test_rpc_timeout.py 8 files changed, 195 insertions(+), 93 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/12049/3 -- To view, visit http://gerrit.cloudera.org:8080/12049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib6007013fc2c9e8eeba11b752ee58fb3038da971 Gerrit-Change-Number: 12049 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4555: Make QueryState's status reporting more robust
Thomas Marshall has posted comments on this change. (
http://gerrit.cloudera.org:8080/12049 )
Change subject: IMPALA-4555: Make QueryState's status reporting more robust
..
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/coordinator-backend-state.cc@300
PS2, Line 300: nstance_exec_status :
> If we send the final report multiple times, won't report_seq_no == last_rep
Yes, but instance_stats->done_ can be set by an intermediate report (if that
particular fragment is done but others aren't). So the following sequence could
occur:
- Some fragment finishes but others are still running, backends sends a report,
backend thinks the report failed but actually the coordinator received it.
- Backend sends another report. This one has a higher sequence number, but
still has the info for the finished fragment as we didn't think that it was
received. Coordinator already processed the report for the finished fragment,
so we want to skip processing it again.
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/coordinator-backend-state.cc@320
PS2, Line 320: n_ran
> const auto&
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/fragment-instance-state.h@101
PS2, Line 101: received
> nit: received
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/query-state.h@387
PS2, Line 387: The number of failed intermediate reports since the la
> This is reset to 0 upon a successful RPC, right ? So, will it be clearer to
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/query-state.cc@386
PS2, Line 386: } else {
> const TUniqueId&
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/be/src/runtime/query-state.cc@563
PS2, Line 563: e all fragment instances finished preparing successfully, start
> Some more thoughts on this:
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/common/protobuf/control_service.proto
File common/protobuf/control_service.proto:
http://gerrit.cloudera.org:8080/#/c/12049/2/common/protobuf/control_service.proto@116
PS2, Line 116: StatefulStatusPB {
> Is there a more succinct name for it ? StatefulStatusPB ?
Done
http://gerrit.cloudera.org:8080/#/c/12049/2/common/protobuf/control_service.proto@138
PS2, Line 138: // Cumulative structural changes made by the table sink of
this fragment
: // instance. This is sent only when 'done' above is true. Not
idempotent.
: optional DmlExecStatusPB dml_exec_status = 5;
> Why isn't this moved into FInstanceExecStatusStatefulPB
Its not quite straight-forward to do, so given this patch is already reasonably
large and complicated, I think its better to leave as followup work.
If you agree, I'll file a JIRA for it.
--
To view, visit http://gerrit.cloudera.org:8080/12049
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6007013fc2c9e8eeba11b752ee58fb3038da971
Gerrit-Change-Number: 12049
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 23 Jan 2019 00:25:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5872: Testcase builder for query planner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12221 ) Change subject: IMPALA-5872: Testcase builder for query planner .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12221/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12221/2/be/src/service/query-options.cc@728 PS2, Line 728: query_options->__set_planner_debug_mode(iequals(value, "true") || iequals(value, "1")); line too long (95 > 90) -- To view, visit http://gerrit.cloudera.org:8080/12221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52 Gerrit-Change-Number: 12221 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 23 Jan 2019 00:24:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5872: Testcase builder for query planner
Hello Greg Rahn, Paul Rogers, Balazs Jeszenszky, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12221 to look at the new patch set (#2). Change subject: IMPALA-5872: Testcase builder for query planner .. IMPALA-5872: Testcase builder for query planner Implements a new testcase builder for simulating query plans from one cluster on a different cluster/minicluster with different number of nodes. The testcase is collected from one cluster and can be replayed on any other cluster. It includes all the information that is needed to replay the query plan exactly as in the source cluster. Also adds a stand-alone tool (PlannerTestCaseLoader) that can replay the testcase without having to start an actual cluster or a dev minicluster. This is done to make testcase debugging simpler. Motivation: -- - Make query planner issues easily reproducible - Improve user experience while collecting query diagnostics - Make it easy to test new planner features by testing it on customer usecases collected from much larger clusters. Commands: -- Collect testcase for a query stmt (outputs the testcase file path). impala-shell> COPY TESTCASE TO -- Load the testcase metadata in a target cluster (dumps the query stmt) impala-shell> COPY TESTCASE FROM -- Replay the query plan impala-shell> SET PLANNER_DEBUG_MODE=true impala-shell> EXPLAIN How it works? - During export on the source cluster, the command dumps all the thrift states of referenced objects in the query into a gzipped binary file. - During replay on a target cluster, it adds these objects to the catalog cache by faking them as DDLs. - The planner also fakes the number of hosts by using the scan range information from the target cluster. Caveats: -- - The tool does not collect actual data files for the tables. Only the metadata state is dumped. - Currently only imports databases/tables/views. We can extend it to work for UDFS etc. - It only works for QueryStmts (select/union queries) - Once the metadata dump is loaded on a target cluster, the state is volatile. Hence it cannot survive a cluster restart / invalidate metadata - Loading a testcase requires setting the query option (SET PLANNER_DEBUG_MODE=true) so that the planner knows to fake the number of hosts. Otherwise it takes into account the local cluster topology. - Cross version compatibility of testcases needs some thought. For example, creating a testcase from Impala version 3.2 and trying to replay it on Impala version 3.5. This could be problematic if we don't keep the underlying thrift structures backward compatible. Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52 --- M be/src/service/client-request-state.cc M be/src/service/query-options.cc M be/src/service/query-options.h M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M common/thrift/CatalogService.thrift M common/thrift/Frontend.thrift M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M common/thrift/JniCatalog.thrift M common/thrift/Types.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java A fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java M fe/src/main/java/org/apache/impala/analysis/HdfsUri.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java A fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 28 files changed, 598 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/12221/2 -- To view, visit http://gerrit.cloudera.org:8080/12221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec83eeb2dc5136768b70ed581fb8d3ed0335cb52 Gerrit-Change-Number: 12221 Gerrit-PatchSet: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Wed, 23 Jan 2019 00:15:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. IMPALA-3323: Fix unrecognizable shell option when --config_file is specified Impala shell defines a dictionary of default values for some shell options. Before this patch, the logic for --config_file checks if a shell option exists by using the default value dictionary, which does not contain the exhaustive list of shell options. This causes a valid option in the Impala shell config file to be treated as unrecognizable shell option due to the option not having a default value. The patch fixes the issue by changing the logic that checks for the existence of an option using the option list from optparse. The patch also fixes the missing dest parameter for ldap_password_cmd option. Testing: - Updated test_shell_commandline::test_config_file - Ran all shell tests Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Reviewed-on: http://gerrit.cloudera.org:8080/12245 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M shell/impala_shell.py M shell/option_parser.py M tests/shell/good_impalarc M tests/shell/test_shell_commandline.py 4 files changed, 23 insertions(+), 12 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-8091: incremental improvements to NTP sync
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12253 ) Change subject: IMPALA-8091: incremental improvements to NTP sync .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc8cacf81765d132dd574993f8149231bdbb7bc6 Gerrit-Change-Number: 12253 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Brown Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 23 Jan 2019 00:02:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8091: incremental improvements to NTP sync
Michael Brown has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12253 Change subject: IMPALA-8091: incremental improvements to NTP sync .. IMPALA-8091: incremental improvements to NTP sync - Warn when ntp-wait is not available. - Install ntp-perl for Redhat-based systems, which provides ntp-wait. Debian-based systems already have ntp-wait as provided by ntp. Change-Id: Ifc8cacf81765d132dd574993f8149231bdbb7bc6 --- M bin/bootstrap_system.sh M testdata/cluster/admin 2 files changed, 4 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/12253/1 -- To view, visit http://gerrit.cloudera.org:8080/12253 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifc8cacf81765d132dd574993f8149231bdbb7bc6 Gerrit-Change-Number: 12253 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Brown
[Impala-ASF-CR] [PROTOTYPE] IMPALA-5872: Test case builder for query planner
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/12221 )
Change subject: [PROTOTYPE] IMPALA-5872: Test case builder for query planner
..
Patch Set 1:
(8 comments)
I'll include a stand-alone planner only Java test in PS2.
> Is there a way to include the query itself in the test case? That eliminates
> the question about "did I get the correct data for the query or query for the
> data?"
It is already there. Included an example output in comments on sql-parser.cup
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/cup/sql-parser.cup@282
PS1, Line 282: KW_ENCODING, KW_END, KW_ESCAPED, KW_EXISTS, KW_EXPLAIN,
KW_EXPORT, KW_EXTENDED,
> Will introducing a keyword using a common term cause existing queries to fa
I redid the syntax to "copy testcase [to|from]" per Greg's suggestion. But yes,
I see your point. Anytime we add new keywords we are at a risk of creating new
incompatibility issues. I think we should document it and suggest the users to
quote them going forward.
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/cup/sql-parser.cup@773
PS1, Line 773: KW_EXPORT KW_TESTCASE KW_INTO KW_OUTFILE STRING_LITERAL:path
query_stmt:query
> Insert a keyword before the query to allow future revisions? Eliminate OUTF
Redid the grammar to copy testcase...(eliminates the outfile keyword). Also
what's the advantage in doing it the json way? Its pretty simple to add but I
don't see the usecase.
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/cup/sql-parser.cup@782
PS1, Line 782: RESULT = new LoadTestCaseStmt(new HdfsUri(path));
> What does LOAD mean? Is this a load-and-plan? Suppose I want to do an EXPLA
It just loads the metadata. The query_stmt is already collected. Sample output
from a load looks like this.
```
[localhost:21000] default> copy testcase from
'hdfs://localhost:20500/tmp/impala-testcase-data-354469c8-1371-4b4e-8978-943018088553';
Query: copy testcase from
'hdfs://localhost:20500/tmp/impala-testcase-data-354469c8-1371-4b4e-8978-943018088553'
.
++
| summary
|
++
| Testcase generated using Impala version 3.2.0-SNAPSHOT. 1 db(s), 2 table(s)
and 2 view(s) imported for query: |
|
|
| SELECT v1.a FROM foo.v1 INNER JOIN foo.v2 ON v1.a = v2.a
|
++```
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java
File fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java:
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java@86
PS1, Line 86: table.getLock().lock();
> Is the lock needed? None of our other table-related code in the analyzer or
That is for Table.toThrift(). So far we had callers for it only in the Catalog
code, hence you don't see this pattern in the analysis code. We had some issues
in the past with multiple threads trying to serialize the table structure
without the lock and that caused some weird data corruption issues. So one of
the fixes added an assert which checks that the current thread holds a lock.
It's annoying but I don't think we can avoid it. (This is another side-effect
of Catalog and analysis sharing Catalog object implementations)
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java@94
PS1, Line 94: if (!result.getDbs().contains(thriftDb)) {
> Does this work? contains() uses the 'equals()' method, which, presumably, i
Done
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java@99
PS1, Line 99: Set allReferencedViews = new HashSet<>();
> This will also be a value set, which means we will compare inline views fie
Guava to the rescue, it has a newIdentityHashSet() which is a wrapper around
IdentityMap.
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/main/java/org/apache/impala/analysis/ExportTestCaseStmt.java@101
PS1, Line 101: for (FeView feView: allReferencedViews) {
> Set iteration has no defined order. Run the same operation twice and the ex
sorted by fullname for deterministic behavior.
http://gerrit.cloudera.org:8080/#/c/12221/1/fe/src/m
[Impala-ASF-CR] IMPALA-7970 : Add support for metastore event based automatic invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12118 ) Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1847/ : 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/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 22 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 22 Jan 2019 22:41:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12250 ) Change subject: IMPALA-7934: Switch to java.util.Base64 implementation .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1846/ : 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/12250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac Gerrit-Change-Number: 12250 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 22 Jan 2019 22:22:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7970 : Add support for metastore event based automatic invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12118 ) Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate .. Patch Set 22: (2 comments) http://gerrit.cloudera.org:8080/#/c/12118/22/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventUtils.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventUtils.java: http://gerrit.cloudera.org:8080/#/c/12118/22/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventUtils.java@337 PS22, Line 337: + "invalidated since it does not exist anymore", getFullyQualifiedTblName())); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/12118/22/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/12118/22/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@443 PS22, Line 443: if (eventsProcessor_.getStatus() != EventProcessorStatus.ACTIVE) eventsProcessor_.start(); line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 22 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 22 Jan 2019 21:47:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7970 : Add support for metastore event based automatic invalidate
Vihang Karajgaonkar has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/12118 ) Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate .. IMPALA-7970 : Add support for metastore event based automatic invalidate This change adds support to CatalogD to poll metastore events to issue invalidate on tables automatically. It adds basic infrastructure to poll HMS notifications events at a configurable frequency using a backend config called hms_event_polling_interval_s flag. Currently, it issues invalidate at tables when it received alter events on table and partitions. It also adds tables/databases and removes tables from catalogD when it receives create_table/create_database and drop_table/drop_database events. The default value of hms_event_polling_interval_s is 0 which disables the feature. A non-zero value in seconds of this configuration can be used to enable the feature and set the polling frequency. In order to process each event atomically, this feature relies on version lock in CatalogServiceCatalog. It adds new methods in CatalogServiceCatalog which takes a write lock on version so that readers are blocked until the catalog state is updated based on the events. In case of processing events, the metastore operation is already completed and only catalog state needs to be updated. Hence we do not need to make new metastore calls while processing the events and only version lock is sufficient to serialize updates to the catalog objects based on events. This locking protocol is similar to what is done in case of DDL processing in CatalogOpExecutor except it does not need to take metastoreDdlLock since no metastore operations are needed during event processing. The change also adds a new test class to test the basic functionality for each of the event type which is supported. Note that this feature is still a work in progress and additional improvements will be done in subsequent patches. By default the feature is turned off. Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java A fe/src/main/java/org/apache/impala/catalog/events/ExternalEventsProcessor.java A fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventUtils.java A fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java A fe/src/main/java/org/apache/impala/catalog/events/MetastoreNotificationException.java A fe/src/main/java/org/apache/impala/catalog/events/NoOpEventProcessor.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java A fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java A fe/src/test/java/org/apache/impala/catalog/events/SynchronousHMSEventProcessorForTests.java M fe/src/test/resources/postgresql-hive-site.xml.template 13 files changed, 1,974 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12118/22 -- To view, visit http://gerrit.cloudera.org:8080/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 22 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-7934: Switch to java.util.Base64 implementation
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12250 Change subject: IMPALA-7934: Switch to java.util.Base64 implementation .. IMPALA-7934: Switch to java.util.Base64 implementation It is shown that java.util.Base64 implementation seems to have better performance compared to Apache Commons Codec's Base64 implementation, which can benefit operations, such as incremental stats. This patch switches the implementation of Base64 from Apache Commons Codec to java.util.Base64 implementation. Testing: - Ran all FE tests Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac --- M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/PartitionStatsUtil.java M fe/src/main/java/org/apache/impala/util/FunctionUtils.java M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java 4 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/12250/2 -- To view, visit http://gerrit.cloudera.org:8080/12250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2d43d4a4f073a800d963ce4c77f21c9efa8471ac Gerrit-Change-Number: 12250 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1845/ : 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/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 21:26:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7979: Enhance decoders to support value-skipping
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12172 ) Change subject: IMPALA-7979: Enhance decoders to support value-skipping .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib848f1bd71735fe84e8064daf700417b32589f57 Gerrit-Change-Number: 12172 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 22 Jan 2019 21:08:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8090: race when reusing ScanRange in test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12238 ) Change subject: IMPALA-8090: race when reusing ScanRange in test .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1844/ : 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/12238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3122e5b2efea60ffe82d780930301d5be108876b Gerrit-Change-Number: 12238 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 22 Jan 2019 21:07:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment or query ids.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 ) Change subject: IMPALA-6664: Tag log statements with fragment or query ids. .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 7 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 22 Jan 2019 20:47:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment or query ids.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 ) Change subject: IMPALA-6664: Tag log statements with fragment or query ids. .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/thread-debug-info.h File be/src/common/thread-debug-info.h: http://gerrit.cloudera.org:8080/#/c/12129/2/be/src/common/thread-debug-info.h@a116 PS2, Line 116: > Thanks! Do you plan to fix it in this change, or in a follow-up commit? Thanks for catching that I didn't do this. I fixed it. Turned out to be nice and easy. http://gerrit.cloudera.org:8080/#/c/12129/5/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/12129/5/be/src/service/impala-hs2-server.cc@126 PS5, Line 126: ScopedThreadContext(GetThreadDebugInfo(), query_ctx.query_id); > Currently it creates a temporary object that is deleted promptly. Whoops. Thanks for catching this. Done. http://gerrit.cloudera.org:8080/#/c/12129/5/be/src/service/impala-internal-service.cc File be/src/service/impala-internal-service.cc: http://gerrit.cloudera.org:8080/#/c/12129/5/be/src/service/impala-internal-service.cc@49 PS5, Line 49: ScopedThreadContext(GetThreadDebugInfo(), params.query_ctx.query_id); > It creates a temporary that is deleted promptly. Done -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 5 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 22 Jan 2019 20:46:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment or query ids.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 ) Change subject: IMPALA-6664: Tag log statements with fragment or query ids. .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3660/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 7 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 22 Jan 2019 20:47:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12120 ) Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1843/ : 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/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Jan 2019 20:44:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7565: Set TAcceptQueueServer connection setup pool to be multi-threaded by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12249 ) Change subject: IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1842/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 Gerrit-Change-Number: 12249 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 22 Jan 2019 20:26:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Jan 2019 20:23:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3659/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Jan 2019 20:23:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3658/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 20:10:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Jan 2019 20:17:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 20:10:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 20:07:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. IMPALA-3323: Fix unrecognizable shell option when --config_file is specified Impala shell defines a dictionary of default values for some shell options. Before this patch, the logic for --config_file checks if a shell option exists by using the default value dictionary, which does not contain the exhaustive list of shell options. This causes a valid option in the Impala shell config file to be treated as unrecognizable shell option due to the option not having a default value. The patch fixes the issue by changing the logic that checks for the existence of an option using the option list from optparse. The patch also fixes the missing dest parameter for ldap_password_cmd option. Testing: - Updated test_shell_commandline::test_config_file - Ran all shell tests Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea --- M shell/impala_shell.py M shell/option_parser.py M tests/shell/good_impalarc M tests/shell/test_shell_commandline.py 4 files changed, 23 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/12245/6 -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py@59 PS3, Line 59: option_list > Note that for 'None' the 'defaults' is not used. Ah, my bad. I thought it was used in the defaults. Done. -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 20:01:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8090: race when reusing ScanRange in test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12238 ) Change subject: IMPALA-8090: race when reusing ScanRange in test .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12238/2/be/src/runtime/io/scan-range.cc File be/src/runtime/io/scan-range.cc: http://gerrit.cloudera.org:8080/#/c/12238/2/be/src/runtime/io/scan-range.cc@240 PS2, Line 240: file_reader_->Close(); > This bug existed earlier as well, but FileReader::Close() is not idempotent Ah good point. The Close() call in CancelInternal() actually means its unsafe to reuse scan ranges that hit an error. I think instead of trying to fix the whole issue I'll document it - product code doesn't reuse scan ranges in this way. -- To view, visit http://gerrit.cloudera.org:8080/12238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3122e5b2efea60ffe82d780930301d5be108876b Gerrit-Change-Number: 12238 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 22 Jan 2019 19:59:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8090: race when reusing ScanRange in test
Hello Zoltan Borok-Nagy, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12238 to look at the new patch set (#3). Change subject: IMPALA-8090: race when reusing ScanRange in test .. IMPALA-8090: race when reusing ScanRange in test The issue occurs in test code where two reads are issued with the same ScanRange object back-to-back, e.g. in SyncReadTest. The DiskIoMgr enqueues the last buffer or cancels the scan range *before* closing the 'file_reader_', which means that the client thread thinks the ScanRange is done and can re-enqueue it into the DiskIoMgr. This is not an issue in Impala itself because scan ranges are not reused in this fashion. I evaluated adding a DCHECK but the lifecycle of the ScanRange objects is complicated enough that there wasn't a straightforward invariant to enforce. Testing: Looped the previously-failing test overnight. Change-Id: I3122e5b2efea60ffe82d780930301d5be108876b --- M be/src/runtime/io/request-ranges.h M be/src/runtime/io/scan-range.cc 2 files changed, 14 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/12238/3 -- To view, visit http://gerrit.cloudera.org:8080/12238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3122e5b2efea60ffe82d780930301d5be108876b Gerrit-Change-Number: 12238 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 5: (1 comment) I still have an issue with the comment, otherwise lgtm. http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py@59 PS3, Line 59: option_list > Done Note that for 'None' the 'defaults' is not used. -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 19:54:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8073: Fix FE tests that leak HMS connections
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12241 ) Change subject: IMPALA-8073: Fix FE tests that leak HMS connections .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12241 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91 Gerrit-Change-Number: 12241 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 22 Jan 2019 19:51:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Hello Pooja Nilangekar, Philip Zeyliger, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12120 to look at the new patch set (#10). Change subject: IMPALA-7941: part 1: detect cgroups memory limit .. IMPALA-7941: part 1: detect cgroups memory limit This adds the logic to detect the cgroups memory limit, but does not use it to set the process memory limit yet. The information obtained is logged at startup and accessible through the Web UI. The patch boils down to reading from the appropriate places in the filesystem. The main complication is that paths need to be translated to point to the right cgroup node inside the container. This deletes some useless cgroup logic from ProcessStateInfo that printed whatever happened to be the last cgroup in a file. Testing: Added a unit test to check that the code could parse the cgroups hierarchy ok and return a positive value. Tested on CentOS6 and CentOS7. Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 --- M be/src/common/init.cc M be/src/util/CMakeLists.txt A be/src/util/cgroup-util.cc A be/src/util/cgroup-util.h M be/src/util/default-path-handlers.cc M be/src/util/proc-info-test.cc M be/src/util/process-state-info.cc M be/src/util/process-state-info.h M www/root.tmpl 9 files changed, 278 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/12120/10 -- To view, visit http://gerrit.cloudera.org:8080/12120 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9 Gerrit-Change-Number: 12120 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8090: race when reusing ScanRange in test
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12238 ) Change subject: IMPALA-8090: race when reusing ScanRange in test .. Patch Set 2: Yeah the lifetime of ScanRange is very unclear: IMPALA-4249 -- To view, visit http://gerrit.cloudera.org:8080/12238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3122e5b2efea60ffe82d780930301d5be108876b Gerrit-Change-Number: 12238 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 22 Jan 2019 19:41:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7941: part 1: detect cgroups memory limit
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/12120 )
Change subject: IMPALA-7941: part 1: detect cgroups memory limit
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:
http://gerrit.cloudera.org:8080/#/c/12120/7/be/src/util/cgroup-util.cc@86
PS7, Line 86: split(fields, line, is_any_of(" "), token_compress_on);
> https://superuser.com/a/527503
Done - used a gutil string escaping function
http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc
File be/src/util/cgroup-util.cc:
http://gerrit.cloudera.org:8080/#/c/12120/9/be/src/util/cgroup-util.cc@55
PS9, Line 55: split(fields, line, is_any_of(":"));
I also checked this and it doesn't look like : is escaped in the paths. Not
sure how much we can reasonably do here:
9:cpu,cpuacct:/user.slice/foo:bar
--
To view, visit http://gerrit.cloudera.org:8080/12120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f5ed0e94511361bf5605822c0874c16c45ffa9
Gerrit-Change-Number: 12120
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 22 Jan 2019 19:37:03 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7565: Set TAcceptQueueServer connection setup pool to be multi-threaded by default
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12249 Change subject: IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default .. IMPALA-7565: Set TAcceptQueueServer connection_setup_pool to be multi-threaded by default Bumping up the thread count for the threads that process the post-accept, pre-setup connection queue to 2 in order to minimize the chances of a single client holding up others if the thread gets stuck in that step. Testing: - Ran exhaustive tests with a thread pool of 10. - Scanned manually through the code and parts of thrift lib to make sure the APIs are used in a thread safe manner. - Rapidly executed openssl connect-disconnect on the impalad's hs2 server port on a thread sanitizer build. No data races were flagged by the thread sanitizer. Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 --- M be/src/rpc/TAcceptQueueServer.cpp 1 file changed, 4 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/12249/1 -- To view, visit http://gerrit.cloudera.org:8080/12249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I053120d4c3153ddbe5261acd28388be6cd191908 Gerrit-Change-Number: 12249 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 ) Change subject: IMPALA-7905: Hive keywords not quoted for identifiers .. Patch Set 11: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@147 PS11, Line 147: true if the identifier is an Impala keyword, or if starts :* with a digit update. -- To view, visit http://gerrit.cloudera.org:8080/12009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0 Gerrit-Change-Number: 12009 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Tue, 22 Jan 2019 18:57:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls
Bharath Vissapragada has abandoned this change. ( http://gerrit.cloudera.org:8080/10696 ) Change subject: [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls .. Abandoned Abandoning for now. Some work has gone into hdfs call timeouts in the form of IMPALA-7738. That should provide some relief for hdfs hangs. Will get back when I find time. -- To view, visit http://gerrit.cloudera.org:8080/10696 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I28e918761c120043d332b034450208eaf34e3e2b Gerrit-Change-Number: 10696 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1841/ : 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/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 17:26:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 ) Change subject: IMPALA-7905: Hive keywords not quoted for identifiers .. Patch Set 11: Pre-review tests now pass again. Please go ahead and re-review. https://jenkins.impala.io/job/pre-review-test/282/ -- To view, visit http://gerrit.cloudera.org:8080/12009 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0 Gerrit-Change-Number: 12009 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Tue, 22 Jan 2019 17:08:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py File shell/option_parser.py: http://gerrit.cloudera.org:8080/#/c/12245/3/shell/option_parser.py@59 PS3, Line 59: option_list > Can you add a comment about the role of option_list? e.g 'option_list' cont Done -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Tue, 22 Jan 2019 16:42:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3323: Fix unrecognizable shell option when --config file is specified
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12245 ) Change subject: IMPALA-3323: Fix unrecognizable shell option when --config_file is specified .. IMPALA-3323: Fix unrecognizable shell option when --config_file is specified Impala shell defines a dictionary of default values for some shell options. Before this patch, the logic for --config_file checks if a shell option exists by using the default value dictionary, which does not contain the exhaustive list of shell options. This causes a valid option in the Impala shell config file to be treated as unrecognizable shell option due to the option not having a default value. The patch fixes the issue by changing the logic that checks for the existence of an option using the option list from optparse. The patch also fixes the missing dest parameter for ldap_password_cmd option. Testing: - Updated test_shell_commandline::test_config_file - Ran all shell tests Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea --- M shell/impala_shell.py M shell/option_parser.py M tests/shell/good_impalarc M tests/shell/test_shell_commandline.py 4 files changed, 24 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/12245/5 -- To view, visit http://gerrit.cloudera.org:8080/12245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iff371d038fa77ba659e9b7c7a4ed5b374237f2ea Gerrit-Change-Number: 12245 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/12247 )
Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc@728
PS3, Line 728: if (iequals(value, "INT96_NANO")) {
> Shouldn't this typo have been caught by a test?
You are right, I think that I messed something up like running the tests with
an older version of impala running (my last change was adding "S" to the end of
query options). I will return soon when the tests pass.
--
To view, visit http://gerrit.cloudera.org:8080/12247
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: Zoltan Ivanfi
Gerrit-Comment-Date: Tue, 22 Jan 2019 16:30:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Zoltan Ivanfi has posted comments on this change. ( http://gerrit.cloudera.org:8080/12247 ) Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc File be/src/exec/parquet/parquet-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc@158 PS3, Line 158: // converted_type is not set because older readers that do not use logical types > I mean I don't see that this function sets the converted type anywhere. Ah, yes, good point, I haven't noticed we are only dealing with LocalDateTime semantics here and regardless of the precision, there is no converted type for that. -- To view, visit http://gerrit.cloudera.org:8080/12247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f Gerrit-Change-Number: 12247 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Tue, 22 Jan 2019 16:01:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12247 ) Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc File be/src/exec/parquet/parquet-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc@158 PS3, Line 158: // converted_type is not set because older readers that do not use logical types > Personally I'm fine with the current name, it sets the converted type when I mean I don't see that this function sets the converted type anywhere. -- To view, visit http://gerrit.cloudera.org:8080/12247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f Gerrit-Change-Number: 12247 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Tue, 22 Jan 2019 15:56:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Zoltan Ivanfi has posted comments on this change. (
http://gerrit.cloudera.org:8080/12247 )
Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc@158
PS3, Line 158: // converted_type is not set because older readers that do not
use logical types
> Then I think you should change the name of the function.
Personally I'm fine with the current name, it sets the converted type when it
makes sense. I don't know how this could be described without making the
function name overly long.
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc@728
PS3, Line 728: if (iequals(value, "INT96_NANO")) {
> It should be "INT96_NANOS"
Shouldn't this typo have been caught by a test?
--
To view, visit http://gerrit.cloudera.org:8080/12247
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: Zoltan Ivanfi
Gerrit-Comment-Date: Tue, 22 Jan 2019 15:44:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/12247 )
Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/hdfs-parquet-table-writer.cc
File be/src/exec/parquet/hdfs-parquet-table-writer.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/hdfs-parquet-table-writer.cc@400
PS3, Line 400: !=
Shouldn't it be '=='?
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/hdfs-parquet-table-writer.cc@583
PS3, Line 583: Overrides of this function are expected to set 'result_'
I think an output parameter would be more conventional.
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/exec/parquet/parquet-metadata-utils.cc@158
PS3, Line 158: // converted_type is not set because older readers that do not
use logical types
Then I think you should change the name of the function.
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/runtime/timestamp-value.inline.h
File be/src/runtime/timestamp-value.inline.h:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/runtime/timestamp-value.inline.h@135
PS3, Line 135: (NANOS_PER_MILLI / 2)
Might be worth to add a short comment about that we do the rounding here.
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/runtime/timestamp-value.inline.h@151
PS3, Line 151: kudu::int128_t nanos128 =
: static_cast(unixtime_seconds) * NANOS_PER_SEC
: + time_.fractional_seconds();
:
: if (nanos128 < std::numeric_limits::min()
: || nanos128 > std::numeric_limits::max()) return
false;
I think we can avoid using int128_t here. We now what are the min and max
unix_timeseconds that can be represented in the limited range.
And if we are at the "boundary" (e.g. 2262-04-11 23:47:16), we now what are the
limits of the fractional_seconds (e.g. 854775807).
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12247/3/be/src/service/query-options.cc@728
PS3, Line 728: if (iequals(value, "INT96_NANO")) {
It should be "INT96_NANOS"
http://gerrit.cloudera.org:8080/#/c/12247/3/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/12247/3/common/thrift/ImpalaInternalService.thrift@1
PS3, Line 1:
nit: accidental new line
--
To view, visit http://gerrit.cloudera.org:8080/12247
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: Zoltan Ivanfi
Gerrit-Comment-Date: Tue, 22 Jan 2019 15:00:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet
Zoltan Ivanfi has posted comments on this change. ( http://gerrit.cloudera.org:8080/12247 ) Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet .. Patch Set 3: Would it be possible to run `parquet-tools dump` as a part of the tests to check whether the timestamp values can be read back correctly by parquet-mr? -- To view, visit http://gerrit.cloudera.org:8080/12247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f Gerrit-Change-Number: 12247 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Ivanfi Gerrit-Comment-Date: Tue, 22 Jan 2019 13:00:31 + Gerrit-HasComments: No
