[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1586/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 6 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 08:05:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1587/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 08:18:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11955 ) Change subject: IMPALA-7844: HAVING clause cannot support ordinals .. Patch Set 4: (1 comment) I like the direction where this is going, however it is a breaking change, therefore I think we can only include it in the next major release. Or, since it was never documented, do we think about the current behavior as some kind of bug? http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java: http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@549 PS4, Line 549: in the list in the predicate? -- To view, visit http://gerrit.cloudera.org:8080/11955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861 Gerrit-Change-Number: 11955 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Dec 2018 16:31:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3557/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 16:53:32 + Gerrit-HasComments: No
[Impala-ASF-CR] Update version to 3.2.0-SNAPSHOT
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12053 ) Change subject: Update version to 3.2.0-SNAPSHOT .. Update version to 3.2.0-SNAPSHOT Change-Id: I69547de6e768470820930fe05f444df416c5f1de Reviewed-on: http://gerrit.cloudera.org:8080/12053 Reviewed-by: Jim Apple Tested-by: Zoltan Borok-Nagy --- M bin/save-version.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jim Apple: Looks good to me, approved Zoltan Borok-Nagy: Verified -- To view, visit http://gerrit.cloudera.org:8080/12053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I69547de6e768470820930fe05f444df416c5f1de Gerrit-Change-Number: 12053 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] Update version to 3.2.0-SNAPSHOT
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12053 ) Change subject: Update version to 3.2.0-SNAPSHOT .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69547de6e768470820930fe05f444df416c5f1de Gerrit-Change-Number: 12053 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Dec 2018 17:21:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11955 ) Change subject: IMPALA-7844: HAVING clause cannot support ordinals .. Patch Set 4: Hmm. I've been thinking about it and since it is not documented and is not supported in other databases, I thought this is a reasonable change. Of course, it is breaking if some users are relying on it. Greg, any thoughts? -- To view, visit http://gerrit.cloudera.org:8080/11955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861 Gerrit-Change-Number: 11955 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Dec 2018 17:46:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Code-Review+1 Fredy, could you +2 when you get a chance? Paul, this probably needs a rebase. -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 12 Dec 2018 18:03:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 12 Dec 2018 18:24:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Paul, can you rebase? -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Wed, 12 Dec 2018 18:25:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. IMPALA-6591: Fix test_ssl flaky test test_ssl has a logic that waits for the number of in-flight queries to be 1. However, the logic for wait_for_num_in_flight_queries(1) only waits for the condition to be true for a period of time and does not throw an exception when the time has elapsed and the condition is not met. In other words, the logic in test_ssl that loops while the number of in-flight queries is 1 never gets executed. I was able to simulate this issue by making Impala shell start much longer. Prior to this patch, in the event that Impala shell took much longer to start, the test started sending the commands to Impala shell even when Impala shell was not ready to receive commands. The patch fixes the issue by waiting until Impala shell is connected. The patch also adds assert in other places that calls wait_for_num_in_flight_queries and updates the default behavior for Impala shell to wait until it is connected. Testing: - Ran core and exhaustive tests several times on CentOS 6 without any issue Change-Id: I9805269d8b806aecf5d744c219967649a041d49f --- M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 6 files changed, 38 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/12047/7 -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py File tests/shell/util.py: http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py@97 PS6, Line 97: expect_success and wait_until_connected) : if expect_success: > This looks equivalent to "expect_success and wait_until_connected". I think Done http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py@138 PS6, Line 138: > You can simplify this to I don't think it's quite the same. When args is None, we also want the shell to wait until it's connected. With args and "--quiet" not in args, the constructor will not wait. http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py@141 PS6, Line 141: connected = "Connected to" in self.shell_process.stderr.readline() > You could also do: Your suggestion is nicer. Done. -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 18:25:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12071/1/docs/topics/impala_authorization.xml File docs/topics/impala_authorization.xml: http://gerrit.cloudera.org:8080/#/c/12071/1/docs/topics/impala_authorization.xml@280 PS1, Line 280: : If you make a change to privileges within Impala, INVALIDATE METADATA : is not required. However, there can be some delay propagating changed privileges from : one impalad to other impalad daemons. : I think we should remove these statements. -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:35:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12071/1/docs/topics/impala_authorization.xml File docs/topics/impala_authorization.xml: http://gerrit.cloudera.org:8080/#/c/12071/1/docs/topics/impala_authorization.xml@280 PS1, Line 280: : If you make a change to privileges within Impala, INVALIDATE METADATA : is not required. However, there can be some delay propagating changed privileges from : one impalad to other impalad daemons. : > I think we should remove these statements. Done -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:40:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/176/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:40:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12071 to look at the new patch set (#2). Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. IMPALA-7728: [DOCS] Added a section on Changing Privileges Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b --- M docs/topics/impala_authorization.xml 1 file changed, 173 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/12071/2 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:44:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12071 to look at the new patch set (#3). Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. IMPALA-7728: [DOCS] Added a section on Changing Privileges Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b --- M docs/topics/impala_authorization.xml 1 file changed, 178 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/12071/3 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/177/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:47:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 12 Dec 2018 18:47:44 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-7924: Add Thrift 0.11.0 to the toolchain
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12034 ) Change subject: IMPALA-7924: Add Thrift 0.11.0 to the toolchain .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb07f43e70844b11ec79c708cab3820628a44cd2 Gerrit-Change-Number: 12034 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 18:48:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 2: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/176/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:50:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/177/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:50:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3558/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 18:52:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 18:52:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 18:53:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7728: [DOCS] Added a section on Changing Privileges
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12071 ) Change subject: IMPALA-7728: [DOCS] Added a section on Changing Privileges .. IMPALA-7728: [DOCS] Added a section on Changing Privileges Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Reviewed-on: http://gerrit.cloudera.org:8080/12071 Tested-by: Impala Public Jenkins Reviewed-by: Alex Rodoni --- M docs/topics/impala_authorization.xml 1 file changed, 178 insertions(+), 128 deletions(-) Approvals: Impala Public Jenkins: Verified Alex Rodoni: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/12071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I955cb49cae24be6a93a90ccb5f2aa6ceb29cee8b Gerrit-Change-Number: 12071 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 7: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py File tests/shell/util.py: http://gerrit.cloudera.org:8080/#/c/12047/6/tests/shell/util.py@138 PS6, Line 138: > I don't think it's quite the same. When args is None, we also want the shel You're right, sry for the mistake. -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 18:45:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1588/ : 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/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 18:55:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7807: Analysis test fixture
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11881 )
Change subject: IMPALA-7807: Analysis test fixture
..
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/AnalysisSessionFixture.java
File fe/src/test/java/org/apache/impala/analysis/AnalysisSessionFixture.java:
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/AnalysisSessionFixture.java@171
PS10, Line 171: public Analyzer analyzer() { return
analysisResult_.getAnalyzer(); }
We need a Preconditions to check if the statement has been analyzed or else it
will result in an NPE.
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@75
PS10, Line 75: public Analyzer analyzer() { return analyzer_; }
Similarly, add Preconditions to check if the statement has been analyzed.
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@78
PS10, Line 78: List rules,
nit: we can probably join L78 with L77
http://gerrit.cloudera.org:8080/#/c/11881/10/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@90
PS10, Line 90:
nit: remove new line
--
To view, visit http://gerrit.cloudera.org:8080/11881
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id69c99b284960f16394f61072d38dd00269bc10c
Gerrit-Change-Number: 11881
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Wed, 12 Dec 2018 19:03:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12036 ) Change subject: IMPALA-7924: Generate Thrift 11 Python Code .. Patch Set 1: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@125 PS1, Line 125: the double word http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@128 PS1, Line 128: this method should be removed. Can you mention IMPALA-7825 here? http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@140 PS1, Line 140: FIL_WE Are FIL and FIL_WE magic strings in CMake? Otherwise, can you find more descriptive names? http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@142 PS1, Line 142: unqiue typo -- To view, visit http://gerrit.cloudera.org:8080/12036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0 Gerrit-Change-Number: 12036 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 19:24:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12024 ) Change subject: IMPALA-7625: test_web_pages.py backend tests are failing .. Patch Set 4: Code-Review+1 (2 comments) I only had some minor comments. http://gerrit.cloudera.org:8080/#/c/12024/4/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/12024/4/tests/common/impala_test_suite.py@837 PS4, Line 837: while (actual_state != expected_state and time.time() - start_time < timeout): nit: you don't need parentheses around the while condition http://gerrit.cloudera.org:8080/#/c/12024/4/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12024/4/tests/webserver/test_web_pages.py@271 PS4, Line 271: cancels the query.""" Should we mention that this function now optionally waits for a particular query state? -- To view, visit http://gerrit.cloudera.org:8080/12024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb Gerrit-Change-Number: 12024 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 12 Dec 2018 19:58:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7948: part 1: initial docker container build
Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/12074 )
Change subject: IMPALA-7948: part 1: initial docker container build
..
Patch Set 4:
(8 comments)
Overall structure seems fine. See some minor comments below. Thanks!
http://gerrit.cloudera.org:8080/#/c/12074/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12074/4//COMMIT_MSG@19
PS4, Line 19: images called "impala_base", "impalad", "catalogd" and
: "statestored":
:
: ninja -j $IMPALA_BUILD_THREADS docker_images
I think it's acceptable to have only one image which takes arguments. Doesn't
matter much one way or another, but it makes life a bit simpler.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile
File docker/impala_base/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile@28
PS4, Line 28: ENV
LD_LIBRARY_PATH=/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/
: ENV
LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/
: ENV LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/etc/kudu/release/lib
You're already using /etc/impala/bin/run_with_classpath.sh in your entrypoint.
It seems like it may make sense to encapsulate the necessary LD_LIBRARY_PATH in
an entrypoint shell script rather than via Docker.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile@31
PS4, Line 31: ENV IMPALA_HOME=/etc/impala
It's not a huge deal, but this feels more like "/opt/impala" to me. i.e., /etc
is just for configs, and it's weird to put the binaries in there.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/run_with_classpath.sh
File docker/run_with_classpath.sh:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/run_with_classpath.sh@33
PS4, Line 33: exit $?
This is usually going to exit gracefully because it's the exit code of the cat.
(Unless the log file doesn't exist...). I think you need to capture the return
code of line 28.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh
File docker/setup_build_context.sh:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@32
PS4, Line 32: echo "Set up a docker build context for the specified
Dockerfile with the impala"\
"Assembles the artifacts required..." as in line 19 is clearer than this
description. ("build context" by itself isn't obvious.) Perhaps use that text
instead?
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@34
PS4, Line 34: echo "-d - set the Dockerfile for the build
(required)"
I think you only ever use this with the "impala_base" dockerfile. Do we
actually need to make this configurable?
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@35
PS4, Line 35: echo "[-o ] - set the output directory (defaults
to"\
Same here. I don't think build directories in our cmake targets are generally
configurable; should we just make this always use the same locations?
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@30
PS4, Line 30: usage() {
: echo "setup_build_context.sh "
: echo "Set up a docker build context for the specified
Dockerfile with the impala"\
:"build artifacts required to produce single-process
containers."
: echo "-d - set the Dockerfile for the build
(required)"
: echo "[-o ] - set the output directory (defaults
to"\
:"docker/build_context)"
: }
:
: # parse command line options
: while [ -n "$*" ]
: do
: case "$1" in
: -o)
: OUTPUT_DIR="${2-}"
: shift;
: ;;
: -d)
: DOCKERFILE="${2-}"
: if [[ "$(basename $DOCKERFILE)" != "Dockerfile" ]]
: then
: echo "$DOCKERFILE must be called Dockerfile"
: usage
: exit 1
: fi
: shift;
: ;;
: -help|*)
: usage
: exit 1
: ;;
: esac
: shift;
: done
This is all fine, but my general rule of thumb is that I regret shell scripts
this long. You'll be happier with python argparse, likely.
--
To view, visit http://gerrit.cloudera.org:8080/12074
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifea707aa3cc23e4facda8ac374160c6de23ffc4e
Gerrit-Change-Number: 12074
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyli
[Impala-ASF-CR] IMPALA-7694: Add host resource usage metrics to profile
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12069 ) Change subject: IMPALA-7694: Add host resource usage metrics to profile .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12069 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aedc20c553ab8d7ed50f72a1a936eba151487d9 Gerrit-Change-Number: 12069 Gerrit-PatchSet: 7 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 12 Dec 2018 20:46:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 12 Dec 2018 20:59:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3559/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 12 Dec 2018 21:10:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 12 Dec 2018 21:10:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12077 Change subject: IMPALA-7351: Add estimates to kudu table sink .. IMPALA-7351: Add estimates to kudu table sink The kudu table sink allocates untracked memory which is bounded by limits that impala enforces through the kudu client API. This patch adds a constant estimate to this table sink which is based on those limits. Testing: Modified planner tests accordingly. Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd --- M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test 4 files changed, 89 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/12077/1 -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12063 Change subject: IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates .. IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates Added justification as comments for UNION, SELECT and Data Source scan nodes that have a constant memory estimate. Change-Id: I0049ea2f50400bd264d9257df0fac7397abb357c --- M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java M fe/src/main/java/org/apache/impala/planner/SelectNode.java M fe/src/main/java/org/apache/impala/planner/UnionNode.java 3 files changed, 24 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/12063/1 -- To view, visit http://gerrit.cloudera.org:8080/12063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0049ea2f50400bd264d9257df0fac7397abb357c Gerrit-Change-Number: 12063 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 ) Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e Gerrit-Change-Number: 12016 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 ) Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3560/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e Gerrit-Change-Number: 12016 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 ) Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code .. Patch Set 2: Code-Review+2 Thanks for the clean up. -- To view, visit http://gerrit.cloudera.org:8080/12016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e Gerrit-Change-Number: 12016 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6964: Track stats about column and page sizes in Parquet reader
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/11575 ) Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader .. Patch Set 14: (12 comments) http://gerrit.cloudera.org:8080/#/c/11575/14/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/11575/14/be/src/exec/hdfs-scan-node-base.h@577 PS14, Line 577: std::unordered_map, std::atomic>> Use our own atomic implementations in common/atomic.h You could also consider using a proper struct instead of the pair to make its usage more readable. http://gerrit.cloudera.org:8080/#/c/11575/14/be/src/exec/parquet/hdfs-parquet-scanner.h File be/src/exec/parquet/hdfs-parquet-scanner.h: http://gerrit.cloudera.org:8080/#/c/11575/14/be/src/exec/parquet/hdfs-parquet-scanner.h@654 PS14, Line 654: Update nit: updates http://gerrit.cloudera.org:8080/#/c/11575/14/be/src/exec/parquet/hdfs-parquet-scanner.h@658 PS14, Line 658: Update nit: updates http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@808 PS14, Line 808: def test_page_size_counters_uncompressed(self, vector): I think you could also pull all (un)compressed tests into one method, e.g. test_per_page_counters_(un)compressed(), but I don't feel strongly about it. http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@822 PS14, Line 822: assert summary['max'] == summary['min'] == summary['avg'] == summary['samples'] == 0 maybe assert all(summary[k] == 0 for k in summary) ? You could also create a helper method for this http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@832 PS14, Line 832: assert summary['max'] > 0 and summary['min'] > 0 and summary['avg'] > 0 and \ maybe assert all(...) here, too? http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@839 PS14, Line 839: + I suspect the + might not be needed here. http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@844 PS14, Line 844: get_bytes_summary_stats_counter(summary_name, runtime_profile) nit: indent 4, or wrap after opening parentheses (see PEP8, I think it says to prefer parentheses for multilines instead of \ when possible) http://gerrit.cloudera.org:8080/#/c/11575/14/tests/query_test/test_scanners.py@859 PS14, Line 859: "ParquetCompressedBytesReadPerColumn", runtime_profile) nit: indent 4, maybe check everywhere else http://gerrit.cloudera.org:8080/#/c/11575/14/tests/util/parse_util.py File tests/util/parse_util.py: http://gerrit.cloudera.org:8080/#/c/11575/14/tests/util/parse_util.py@152 PS14, Line 152: .*? This matches non-greedily until the opening (, right? You can also use [^\(] instead, might be slightly clear but I don't feel strongly. http://gerrit.cloudera.org:8080/#/c/11575/14/tests/util/parse_util.py@152 PS14, Line 152: [.]*[0-9]* Then this only needs to match the number in (), right? so we don't need the decimal and numbers after. http://gerrit.cloudera.org:8080/#/c/11575/14/tests/util/parse_util.py@155 PS14, Line 155: * Nit: here and elsewhere you likely mean "+" (1 or more) instead of "*" (zero or more) -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 14 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 12 Dec 2018 21:49:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/11517 ) Change subject: [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files .. Patch Set 3: The discussion looks like it's not easy to add the tests on HDFS. Should we try S3 then? -- To view, visit http://gerrit.cloudera.org:8080/11517 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I211e2511ea3bb5edea29f1bd63e6b1fa4c4b1965 Gerrit-Change-Number: 11517 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 21:53:54 + Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-7178: Add the possibility to reduce logging for common data errors
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10793 ) Change subject: WIP IMPALA-7178: Add the possibility to reduce logging for common data errors .. Patch Set 1: Any updates? Should we abandon this? -- To view, visit http://gerrit.cloudera.org:8080/10793 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3b7c1fd020a7ba5e0d9c619e1b67236dce198aa Gerrit-Change-Number: 10793 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 12 Dec 2018 21:54:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1589/ : 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/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 22:05:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12063 ) Change subject: IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1590/ : 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/12063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0049ea2f50400bd264d9257df0fac7397abb357c Gerrit-Change-Number: 12063 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 22:14:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7047. Refreshing partitions should not make an RPC per file
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11227 ) Change subject: IMPALA-7047. Refreshing partitions should not make an RPC per file .. Patch Set 7: Had a chat with Paul offline. Appears that there is a test gap and he is looking into fixing it. -- To view, visit http://gerrit.cloudera.org:8080/11227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2051b96599206164aaa06ecbdf64374c46eda956 Gerrit-Change-Number: 11227 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 12 Dec 2018 22:08:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 22:35:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. IMPALA-6591: Fix test_ssl flaky test test_ssl has a logic that waits for the number of in-flight queries to be 1. However, the logic for wait_for_num_in_flight_queries(1) only waits for the condition to be true for a period of time and does not throw an exception when the time has elapsed and the condition is not met. In other words, the logic in test_ssl that loops while the number of in-flight queries is 1 never gets executed. I was able to simulate this issue by making Impala shell start much longer. Prior to this patch, in the event that Impala shell took much longer to start, the test started sending the commands to Impala shell even when Impala shell was not ready to receive commands. The patch fixes the issue by waiting until Impala shell is connected. The patch also adds assert in other places that calls wait_for_num_in_flight_queries and updates the default behavior for Impala shell to wait until it is connected. Testing: - Ran core and exhaustive tests several times on CentOS 6 without any issue Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Reviewed-on: http://gerrit.cloudera.org:8080/12047 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_client_ssl.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 6 files changed, 38 insertions(+), 21 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-6591: Fix test ssl flaky test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12047 ) Change subject: IMPALA-6591: Fix test_ssl flaky test .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9805269d8b806aecf5d744c219967649a041d49f Gerrit-Change-Number: 12047 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Wed, 12 Dec 2018 22:44:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Yongjun Zhang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. IMPALA-5474: Adding a trivial subquery turns error into warning After adding a subquery to a query that fails with ERROR, it fails with WARNING. The fix here makes it return ERROR. Testing: Added unit tests; Done real cluster testing with reported cases. Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e --- M shell/impala_client.py M tests/shell/test_shell_commandline.py 2 files changed, 33 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/12022/4 -- To view, visit http://gerrit.cloudera.org:8080/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Yongjun Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. Patch Set 4: Thanks much for the review and comments Tim! I added comments to the new methods. -- To view, visit http://gerrit.cloudera.org:8080/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Wed, 12 Dec 2018 23:09:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3561/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Wed, 12 Dec 2018 23:11:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 23:41:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1591/ : 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/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Wed, 12 Dec 2018 23:42:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 431 insertions(+), 197 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/12 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 12:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS11, Line 138: Preconditions.checkNotNull(tableName_);
> Preconditions.checkNotNull(tableName_) between L137-138
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS8, Line 1140: blic void refreshAuthorizat
> Can we reverse it then. if (sentryProxy == null) return;
Done.
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3451
PS8, Line 3451: List added = new ArrayList<>();
> Actually thinking a bit more about this, I still am not convinced. I think
Thanks for the explanation, that makes sense. I have updated the code to keep
track of list of catalog objects added and removed.
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@546
PS8, Line 546:
> Can we also do execute_query_expect_failure(select from functional.alltypes
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@555
PS8, Line 555: assert any("select" in x for x in result.data)
> execute_query_expect_success()
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py@211
PS8, Line 211: update.
: """
: db_name = "test_role_privilege_case_x_" + get_random_id(5)
: db_name_upper_case = "TEST_ROLE_PRIVILEGE_CASE_Y_" +
get_random_id(5).upper()
: db_name_mixed_case = "TesT_Role_PRIVIlege_case_z" +
get_random_id(5)
: role_name = "test_role_" + get_random_id(5)
: try:
: self.client.execute("create role {0}".format(role_name))
: self.client.execute("grant all on server to
{0}".format(role_name))
: self.client.execute("grant role {0} to group
`{1}`".format(role_name,
: grp.getgrnam(getuser()).gr_name))
:
: self.client.execute("create database " + db_name)
: self.client.execute("create database " +
db_name_upper_case)
: self.client.execute("create database " +
db_name_mixed_case)
: self.client.execute(
: "create table if not exists {0}.test1(i
int)".format(db_name))
: self.client.execute("create table if not exists
{0}.TEST2(i int)".format(db_name))
: self.client.execute("create table if not exists
{0}.Test3(i int)".format(db_name))
:
: self.client.execute(
: "grant select on table {0}.test1 to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant select on table {0}.TEST2 to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant select on table {0}.TesT3 to
{1}".format(db_name, role_name))
: self.client.execute("grant all on database {0} to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant all on database {0} to
{1}".format(db_name_upper_case, role_name))
: self.client.execute(
: "grant all on database {0} to
{1}".format(db_name_mixed_case, role_name))
: result = self.client.execute("show grant role
{0}".format(role_name))
: assert any('test1' in x for x in result.data)
: assert any('test2' in x for x in result.data)
: assert any('test3' in x for x in result.data)
> can we fold this into test_sentry_refresh?
Yeah this actually looks very similar to test_refresh_authorization. I don't
know why I put this test here :(
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 23:47:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3562/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 12 Dec 2018 23:47:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/12001 )
Change subject: IMPALA-7902: NumericLiteral fixes, refactoring
..
Patch Set 10: Code-Review+1
(3 comments)
The code flow seems fine to me, specially around NumericLiteral's life-cycle of
implicit and explicit types. I'm not familiar with the intricacies of type
overflows and how they are handled. So I'm not confident enough to +2 this.
Since Tim already took a look, I think you can upgrade to a +2 once the nits
are fixed.
http://gerrit.cloudera.org:8080/#/c/12001/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/12001/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@234
PS10, Line 234: catch (SqlCastException e) {
: return null;
: }
I see that you are trying to preserve the earlier behavior here. But doesn't it
make sense to propagate the SqlCastException for overflow? Just curious (I'm
not sure why we returned null earlier)
http://gerrit.cloudera.org:8080/#/c/12001/10/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
File fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java:
http://gerrit.cloudera.org:8080/#/c/12001/10/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java@52
PS10, Line 52: >
nit: ?
http://gerrit.cloudera.org:8080/#/c/12001/10/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java@113
PS10, Line 113: /**
nit: use // style comments for vars
--
To view, visit http://gerrit.cloudera.org:8080/12001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2
Gerrit-Change-Number: 12001
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 13 Dec 2018 00:17:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1592/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 13 Dec 2018 00:21:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12068 ) Change subject: IMPALA-7657: Codegen IsNotEmptyPredicate and ValidTupleIdExpr. .. Patch Set 3: I tested performance of the two scalar expressions by running queries on my desktop box. ValidTupleId is faster with the new codegen. To prove this I used a query with many 'sum(distinct column_name)' clauses. In this case the new code runs 2% faster. I think this would be undetectable in any normal query. I ran many queries trying to analyze whether IsNotEmptyPredicate is faster with the new codegen. In most queries using the predicate there was no detectable difference. I was able to construct a pathological query with 100 union all expressions, each of which read from a nested type. With this query runtime was 5% slower with the new codegen. I don't think this would be detectable in any normal query. Arguably we should still use this new codegen'd version as it fits with our vision of removing all calls to GetCodegendComputeFnWrapper(). -- To view, visit http://gerrit.cloudera.org:8080/12068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb87b9e3b879c278ce8638d97bcb320a7555a6b3 Gerrit-Change-Number: 12068 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 13 Dec 2018 00:21:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 13 Dec 2018 01:08:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7926: Fix flakiness in test reconnect
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12045 ) Change subject: IMPALA-7926: Fix flakiness in test_reconnect .. IMPALA-7926: Fix flakiness in test_reconnect test_reconnect launches a shell that connects to one impalad in the minicluster then reconnects to a different impalad while checking that the impalad's open session metric changes accordingly. To do this, the test gets the number of open sessions at the start of the test and then expects that the number of sessions will have increased by 1 on the impalad that the shell is currently connected to. This can be a problem if there is a session left over from another test that is still active when test_reconnect starts but exits while it's running. test_reconnect is already marked to run serially, so there shouldn't be any other sessions open while it runs anyways. The solution is to wait at the start of the test until any sessions left over from other tests have exited. Testing: - Ran the test in an environment where the timing was previously causing it to fail almost deterministically and it now passes. Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Reviewed-on: http://gerrit.cloudera.org:8080/12045 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/shell/test_shell_interactive.py 1 file changed, 17 insertions(+), 15 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3017ca3bf7b4e33440cffb80e9a48a63bec14434 Gerrit-Change-Number: 12045 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 ) Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3560/ -- To view, visit http://gerrit.cloudera.org:8080/12016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e Gerrit-Change-Number: 12016 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 13 Dec 2018 01:29:06 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py@24 PS1, Line 24: import json flake8: F401 'json' imported but unused http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py@375 PS1, Line 375: def wait_for_cluster(timeout_in_seconds=CLUSTER_WAIT_TIMEOUT_IN_SECONDS): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 13 Dec 2018 01:45:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12078 Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Remove Python 2.4 workarounds in start-impala-cluster.py The workarounds are: * Trying to handle the json module not being present. That module was added in Python 2.6 * Trying to handle import failure of ImpalaCluster. It's less clear why this was necessary, but I can't see a reason why we wouldn't be able to import our own code that is used in many places in the tests. Neither seems necessary at this point. Testing: Ran a build on CentOS 6 with Python 2.4 and confirmed that it was able to start the cluster OK. Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c --- M bin/start-impala-cluster.py 1 file changed, 9 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/12078/1 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py@24 PS1, Line 24: import json > flake8: F401 'json' imported but unused Done http://gerrit.cloudera.org:8080/#/c/12078/1/bin/start-impala-cluster.py@375 PS1, Line 375: def wait_for_cluster(timeout_in_seconds=CLUSTER_WAIT_TIMEOUT_IN_SECONDS): > flake8: E302 expected 2 blank lines, found 1 Done -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 01:47:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Hello Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12078 to look at the new patch set (#2). Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Remove Python 2.4 workarounds in start-impala-cluster.py The workarounds are: * Trying to handle the json module not being present. That module was added in Python 2.6 and also isn't used in this file! * Trying to handle import failure of ImpalaCluster. It's less clear why this was necessary, but I can't see a reason why we wouldn't be able to import our own code that is used in many places in the tests. Neither seems necessary at this point. Testing: Ran a build on CentOS 6 with Python 2.4 and confirmed that it was able to start the cluster OK. Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c --- M bin/start-impala-cluster.py 1 file changed, 9 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/12078/2 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals
Hello Bharath Vissapragada, Greg Rahn, Zoltan Borok-Nagy, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11955 to look at the new patch set (#5). Change subject: IMPALA-7844: HAVING clause cannot support ordinals .. IMPALA-7844: HAVING clause cannot support ordinals The SELECT statement has two clauses that take lists of columns and/or aliases: ORDER BY and GROUP BY. Each element is a alias, a table.column reference or a number, which represents the ordinal number of a column. Thus, "GROUP BY a, 2, c" is unambiguous. SELECT also has a number of predicate clauses: WHERE and HAVING. In these, aliases are possible (though seldom suppored), but ordinals are ambiguous: is "WHERE 1 = 2" a reference to two constants, two columns by ordinal, or a combination? No SQL dialect supports ordinals in WHERE or HAVING for this reason. Impala seems to have adopted a rather odd convention: if the HAVING predicate has only one element (no operators), then that one element can be an ordinal or alias. Thus "HAVING a" and "HAVING 1" are valid, only if alias a or the column at ordinal 1 are BOOLEAN. However, "HAVING a = 1" and "HAVING 1 = 2" are not valid. This is unusual because HAVING is normally a predicate: "HAVING a = 10". This fix prepares to remove the attempt to support ordinals in the HAVING clause, after which Impala will treat HAVING like WHERE, rather than trying to treat it like ORDER BY or GROUP BY. Review comments suggest that such a breaking change (even for such a non-standard, undocumented, odd feature) should occur only in a major version. So, for now, the no-ordinal support can be enabled via a constant, but is turned off by default. (Perhaps a later patch can add a session or runtime option instead of the constant.) The fix retains the limited form of alias support. Refactored the "ordinal or alias" code to make alias resolution optional. Reworded a few error messages for greater clarity. Testing: * Refactored AnalyzeStmtsTest to split apart the alias and ordinals cases for easier debugging. * Tests can validate either the current HAVING ordinal behavior or the proposed new behavior. Test path is controlled by the constant described above. Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861 --- M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java 5 files changed, 324 insertions(+), 142 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/11955/5 -- To view, visit http://gerrit.cloudera.org:8080/11955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861 Gerrit-Change-Number: 11955 Gerrit-PatchSet: 5 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/11955 )
Change subject: IMPALA-7844: HAVING clause cannot support ordinals
..
Patch Set 4:
(10 comments)
Revised based on review comments. Rebased on master.
Thanks for the suggestion that we want to wait until a major release to disable
ordinals. Fortunately, it was easy to add a constant that controls the feature:
set it to enable an ordinal in HAVING by default, to avoid breaking anything.
At the major release, the constant can transform into a feature flag that can
be enabled in the unlikely case anyone actually uses ordinals in HAVING.
Tests are conditional on the feature setting.
The refactoring is still needed for other work, so please do review the updated
code.
http://gerrit.cloudera.org:8080/#/c/11955/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11955/4//COMMIT_MSG@21
PS4, Line 21: opeators
> typo
Done
http://gerrit.cloudera.org:8080/#/c/11955/4//COMMIT_MSG@21
PS4, Line 21: than
> then
Done
http://gerrit.cloudera.org:8080/#/c/11955/4//COMMIT_MSG@24
PS4, Line 24: unusal
> typo
Done
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@329
PS4, Line 329: resoluton
> typo
Done
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@336
PS4, Line 336: Substitutes
> Not sure if the substitution is happening here. As I understand it, we just
I agree that this code is confusing. Here's my understanding:
This bit of code replaces alias and ordinals (both of which are references to a
SELECT list item). In SQL, both are semi-equivalent ways to refer to result set
columns.
For historical reasons, we allow alias only as the only item in an expression.
(Works for ORDER BY, not for HAVING.)
This code replaces the ordinal/alias with the result set *expression*, which is
surprising. Most systems replace it with a reference to that column (as "slot
ref" in Impala terminology.)
Later, we use an "smap" (substation map) to match the expression with a result
set expression. While odd, this does unify the cases of a) ordinal, b) column
alias, and c) repeating the result set expression.
The main change here is to recognize that ordinals work in lists (ORDER BY) but
not expressions (HAVING).
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@353
PS4, Line 353: allowOrdinal
> mention this in the method doc
Done
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@389
PS4, Line 389: Analyze it so all expressions exit
: // this method analyzed.
> Isn't this happening L372?
Done
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@549
PS4, Line 549: in the list
> in the predicate?
Done
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@1074
PS4, Line 1074: "if(true, 7, int_col)");
> Test -ve ordinal values?
Sorry, -ve?
http://gerrit.cloudera.org:8080/#/c/11955/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@1124
PS4, Line 1124: @Ignore("IMPALA-7844: Ordinals not supported in HAVING")
> Don't think this is the right way. Instead, convert them into proper Analys
Revised. Given the hesitancy to change the behavior now, preserved original
behavior, gated by a constant. Used that constant to choose the new tests for
HAVING above, or the "legacy" tests here.
--
To view, visit http://gerrit.cloudera.org:8080/11955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
Gerrit-Change-Number: 11955
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Greg Rahn
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 13 Dec 2018 02:04:57 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1593/ : 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/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 02:20:11 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 02:26:27 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1594/ : 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/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 02:28:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11955 ) Change subject: IMPALA-7844: HAVING clause cannot support ordinals .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1595/ : 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/11955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861 Gerrit-Change-Number: 11955 Gerrit-PatchSet: 5 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 13 Dec 2018 02:36:07 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 02:49:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7948: part 1: initial docker container build
Hello Philip Zeyliger, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12074 to look at the new patch set (#5). Change subject: IMPALA-7948: part 1: initial docker container build .. IMPALA-7948: part 1: initial docker container build This builds an impala_base container that has all of the build artifacts required to run the impala processes, then builds impalad, catalogd and statestore containers based on that with the right ports exposed. The images are based on the Ubuntu 16.04 image to align with the most common development environment. The container build process is integrated with CMake and is designed to integrate with the rest of the build so that the container build depends on the artifacts that will go into the container. You can build the images with the following command, which will create images called "impala_base", "impalad", "catalogd" and "statestored": ninja -j $IMPALA_BUILD_THREADS docker_images The images need some refinement to be truly useful. The following will be done in future patches: * IMPALA-7947 - integrate with start-impala-cluster.py to automatically create docker network with containers running on it * Mechanism to pass in command-line flags * Mechanisms to update the various config files to point to the docker host rather than "localhost", which doesn't point to the right thing inside the container. * Mechanisms to set mem_limit, JVM heap sizes, etc, automatically. * Mapping /etc/localtime from host Testing: Manually started up the containers connected to a user-defined bridge network, tweaked the configurations to point to the HMS/HDFS/etc running on my host. I then used "docker ps" to figure out the port mappings for beeswax and debug webserver. Confirmed that I could run a query and access debug pages: $ impala-shell.sh -i localhost:32860 -q "select coordinator()" Starting Impala Shell without Kerberos authentication Opened TCP connection to localhost:32860 Connected to localhost:32860 Server version: impalad version 3.1.0-SNAPSHOT DEBUG (build d7870fe03645490f95bd5ffd4a2177f90eb2f3c0) Query: select coordinator() Query submitted at: 2018-12-11 15:51:04 (Coordinator: http://8063e77ce999:25000) Query progress can be monitored at: http://8063e77ce999:25000/query_plan?query_id=1b4d03f0f0f1fcfb:b0b37e50 +---+ | coordinator() | +---+ | 8063e77ce999 | +---+ Fetched 1 row(s) in 0.11s Change-Id: Ifea707aa3cc23e4facda8ac374160c6de23ffc4e --- M .gitignore M CMakeLists.txt M be/src/service/CMakeLists.txt M bin/start-catalogd.sh M bin/start-statestored.sh A docker/CMakeLists.txt M docker/README.md A docker/catalogd/Dockerfile A docker/daemon_entrypoint.sh A docker/impala_base/Dockerfile A docker/impalad/Dockerfile A docker/setup_build_context.py A docker/statestored/Dockerfile M fe/CMakeLists.txt 14 files changed, 380 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/12074/5 -- To view, visit http://gerrit.cloudera.org:8080/12074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifea707aa3cc23e4facda8ac374160c6de23ffc4e Gerrit-Change-Number: 12074 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7948: part 1: initial docker container build
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/12074 )
Change subject: IMPALA-7948: part 1: initial docker container build
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/12074/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12074/4//COMMIT_MSG@19
PS4, Line 19: images called "impala_base", "impalad", "catalogd" and
: "statestored":
:
: ninja -j $IMPALA_BUILD_THREADS docker_images
> I think it's acceptable to have only one image which takes arguments. Doesn
I ended up going down this path because the set of public-facing ports exposed
by each daemon is different. I suppose we could expose the superset of all
ports, but that seems to be not the way things are usually done in Docker - I
think?
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile
File docker/impala_base/Dockerfile:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile@28
PS4, Line 28: ENV
LD_LIBRARY_PATH=/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/
: ENV
LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server/
: ENV LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/etc/kudu/release/lib
> You're already using /etc/impala/bin/run_with_classpath.sh in your entrypoi
Fair point. Converted to a daemon_entrypoint.sh script
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/impala_base/Dockerfile@31
PS4, Line 31: ENV IMPALA_HOME=/etc/impala
> It's not a huge deal, but this feels more like "/opt/impala" to me. i.e., /
Done
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/run_with_classpath.sh
File docker/run_with_classpath.sh:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/run_with_classpath.sh@33
PS4, Line 33: exit $?
> This is usually going to exit gracefully because it's the exit code of the
Done
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh
File docker/setup_build_context.sh:
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@32
PS4, Line 32: echo "Set up a docker build context for the specified
Dockerfile with the impala"\
> "Assembles the artifacts required..." as in line 19 is clearer than this de
I removed the arg parsing so this is not longer present.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@34
PS4, Line 34: echo "-d - set the Dockerfile for the build
(required)"
> I think you only ever use this with the "impala_base" dockerfile. Do we act
Yeah I think I made this more general than necessary in anticipation of
requiring build contexts for the other containers. Removed.
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@35
PS4, Line 35: echo "[-o ] - set the output directory (defaults
to"\
> Same here. I don't think build directories in our cmake targets are general
Done
http://gerrit.cloudera.org:8080/#/c/12074/4/docker/setup_build_context.sh@30
PS4, Line 30: usage() {
: echo "setup_build_context.sh "
: echo "Set up a docker build context for the specified
Dockerfile with the impala"\
:"build artifacts required to produce single-process
containers."
: echo "-d - set the Dockerfile for the build
(required)"
: echo "[-o ] - set the output directory (defaults
to"\
:"docker/build_context)"
: }
:
: # parse command line options
: while [ -n "$*" ]
: do
: case "$1" in
: -o)
: OUTPUT_DIR="${2-}"
: shift;
: ;;
: -d)
: DOCKERFILE="${2-}"
: if [[ "$(basename $DOCKERFILE)" != "Dockerfile" ]]
: then
: echo "$DOCKERFILE must be called Dockerfile"
: usage
: exit 1
: fi
: shift;
: ;;
: -help|*)
: usage
: exit 1
: ;;
: esac
: shift;
: done
> This is all fine, but my general rule of thumb is that I regret shell scrip
I did the rewrite. It wasn't quite mechanical but wasn't particularly tricky.
I removed the argument parsing and assumed fixed paths for now.
--
To view, visit http://gerrit.cloudera.org:8080/12074
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifea707aa3cc23e4facda8ac374160c6de23ffc4e
Gerrit-Change-Number: 12074
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Com
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3563/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 02:49:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3561/ -- To view, visit http://gerrit.cloudera.org:8080/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 13 Dec 2018 03:11:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7948: part 1: initial docker container build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12074 ) Change subject: IMPALA-7948: part 1: initial docker container build .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1596/ : 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/12074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifea707aa3cc23e4facda8ac374160c6de23ffc4e Gerrit-Change-Number: 12074 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 03:41:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 03:46:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351: Add estimates to kudu table sink
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12077 ) Change subject: IMPALA-7351: Add estimates to kudu table sink .. IMPALA-7351: Add estimates to kudu table sink The kudu table sink allocates untracked memory which is bounded by limits that impala enforces through the kudu client API. This patch adds a constant estimate to this table sink which is based on those limits. Testing: Modified planner tests accordingly. Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Reviewed-on: http://gerrit.cloudera.org:8080/12077 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/planner/KuduTableSink.java M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test 4 files changed, 89 insertions(+), 23 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I89a45dce0cfbbe3cc0bc17d55ffdbd41cd7dbfbd Gerrit-Change-Number: 12077 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Hello Bharath Vissapragada, Fredy Wijaya, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12001 to look at the new patch set (#11). Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. IMPALA-7902: NumericLiteral fixes, refactoring The work to clean up the rewriter logic must start with a stable AST, which must start with sprucing up some issues with the leaf nodes. This CR tackles the NumericLiteral used to hold numbers. IMPALA-7896: Literals should not need explicit analyze step Partial fix: removes the need to analyze a numeric literal: analyze() is a no-op. This eliminates the need to do a "fake" analysis with a null analyzer: numeric literals are now created analyzed. This is useful because the catalog module creates numeric literals outside of a query (and outside of an analyzer.) A literal is immutable except for type. Modified the constructor to set the type and cost, then mark the node as analyzed. A later call to analyze() has nothing to do. Code that created and dummy-analyzed numeric literals changed to use static create() methods resulting in simpler literal creation, and eliminates the special "analyzer == null" checks in analyze(). IMPALA-7886: NumericLiteral constructor fails to round values to Decimal type IMPALA-7887: NumericLiteral fails to detect numeric overflow IMPALA-7888: Incorrect NumericLiteral overflow checks for FLOAT, DOUBLE IMPALA-7891: Analyzer does not detect numeric overflow in CAST IMPALA-7894: Parser does not catch double overflow These are all caused by the somewhat cluttered state of the numeric range check code after years of incremental changes. This patch centralizes all checks into a series of constants and methods for uniformity. All values are set in the constructor which now checks that the value is legal for the type. Cast operations verify that the cast is valid. Multiple semi-parallel versions of the same logic is replaced by calls to a single implementation. The numeric checks now follow the SQL standard which says that implementations should fail if a cast would trucate the most significant digits, but round when truncating the least significant. IMPALA-7865: Repeated type widening of arithmetic expressions Partial fix. Replaces the "is explicit cast" flag in the numeric literal with the explicit type. This allows reseting an implicit type back to the explciit type if an arithmetic expression is analyzed multiple times. A later patch will feed this type information into the type inference mechanism to complete the fix. Finally, adds a set of new exceptions that begin to unify error reporting. These handle casts (SqlCastException), value validation (InvalidValueException) and unsupported features (UnsupportedFeatureException.) These all derive from AnalysisException for backward compatibility. Tests use the new exceptions to check for expected errors rather than parsing text strings (which tend to change.) Testing: * Added unit tests just for numeric literals. Refactored code to simplify the tests. * Added a test case for the obscure case in Decimal V1 of an implicit cast overflow. * The depth-check tests needed one extra level of nesting to trigger failure. * Ran all FE tests. Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/ScalarType.java A fe/src/main/java/org/apache/impala/common/InvalidValueException.java A fe/src/main/java/org/apache/impala/common/SqlCastException.java A fe/src/main/java/org/apache/impala/common/UnsupportedFeatureException.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java D fe/src/test/java/org/apache/impala/analysis/ExprTest.java A fe/src/test/java/org/apache/impala/analysis/LiteralExprTest.java A fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/HdfsPartitionTest.java 19 files changed, 1,390 insertions(+), 283 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12001/11 -- To view, visit http://gerrit.cloudera.org:8080/12001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Bra
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/12016 )
Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
..
Patch Set 3:
Failure seems to be in one of the Python tests:
=== FAILURES ===
___ TestImpalaShellInteractive.test_malformed_query
[gw14] linux2 -- Python 2.7.12
/home/ubuntu/Impala/bin/../infra/python/env/bin/python
shell/test_shell_interactive.py:725: in test_malformed_query
assert "ERROR: AnalysisException: Unmatched string literal" in result.stderr
E assert 'ERROR: AnalysisException: Unmatched string literal' in "Starting
Impala Shell without Kerberos authentication\nOpened TCP connection to
localhost:21000\nConnected to localho...select foo(''), ('bar\n
^\n\nCAUSED BY: Exception: Unmatched string literal\n\nGoodbye ubuntu\n"
E+ where "Starting Impala Shell without Kerberos authentication\nOpened
TCP connection to localhost:21000\nConnected to localho...select foo(''),
('bar\n^\n\nCAUSED BY: Exception: Unmatched string
literal\n\nGoodbye ubuntu\n" = .stderr
Anybody know about this one?
--
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Thu, 13 Dec 2018 04:00:53 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Thanks much for the reviews. Rebased on latest master. -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 13 Dec 2018 04:02:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12001 ) Change subject: IMPALA-7902: NumericLiteral fixes, refactoring .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1597/ : 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/12001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 Gerrit-Change-Number: 12001 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-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 04:21:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5474: Adding a trivial subquery turns error into warning
Yongjun Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12022 ) Change subject: IMPALA-5474: Adding a trivial subquery turns error into warning .. Patch Set 4: > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3561/ The failed test https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/3863/ appears to be not caused by my change. This new rev only has some additional comments added, comparing with last rev that had successful test run. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/12022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibedb11dd3d50bcdb21d508f7d21691925491946e Gerrit-Change-Number: 12022 Gerrit-PatchSet: 4 Gerrit-Owner: Yongjun Zhang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Yongjun Zhang Gerrit-Comment-Date: Thu, 13 Dec 2018 04:45:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7867 (Part 2): ArrayList cleanup in analyzer
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11995 ) Change subject: IMPALA-7867 (Part 2): ArrayList cleanup in analyzer .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3565/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11995 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c0b5f40a0504fc2d324055bd2a962e35f8e744a Gerrit-Change-Number: 11995 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 13 Dec 2018 06:03:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12063 ) Change subject: IMPALA-7351 IMPALA-7353: Add justification for constant memory estimates .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0049ea2f50400bd264d9257df0fac7397abb357c Gerrit-Change-Number: 12063 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 06:16:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 ) Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code .. Patch Set 3: That looks related to your change. AnalysException -> ParseException stuff. -- To view, visit http://gerrit.cloudera.org:8080/12016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e Gerrit-Change-Number: 12016 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Rogers Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 13 Dec 2018 06:32:53 + Gerrit-HasComments: No
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Remove Python 2.4 workarounds in start-impala-cluster.py The workarounds are: * Trying to handle the json module not being present. That module was added in Python 2.6 and also isn't used in this file! * Trying to handle import failure of ImpalaCluster. It's less clear why this was necessary, but I can't see a reason why we wouldn't be able to import our own code that is used in many places in the tests. Neither seems necessary at this point. Testing: Ran a build on CentOS 6 with Python 2.4 and confirmed that it was able to start the cluster OK. Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Reviewed-on: http://gerrit.cloudera.org:8080/12078 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/start-impala-cluster.py 1 file changed, 9 insertions(+), 40 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Remove Python 2.4 workarounds in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12078 ) Change subject: Remove Python 2.4 workarounds in start-impala-cluster.py .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia28a7b75f2e2804834e5df57ef7998b360112e5c Gerrit-Change-Number: 12078 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 13 Dec 2018 06:52:45 + Gerrit-HasComments: No
