[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/96/ : 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/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 28 Jul 2018 03:56:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
..
Patch Set 9:
(5 comments)
Lol, hopefully with some refactoring it's a little more readable - I did just
put it together quickly
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@863
PS9, Line 863: void SimpleTupleStreamTest::TestFlushResourcesReadWrite(bool
pin_stream, bool attach_on_read) {
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@913
PS9, Line 913: for (int j = 0; j < out_batch->num_rows(); ++j) {
> style: Level 5 loop nesting seems a bit too deep to me - can you move some
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@916
PS9, Line 916:
*static_cast(out_batch->GetRow(j)->GetTuple(0)->GetSlot(slot_offset)));
> nit: long line
Done
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h@395
PS9, Line 395: AttachBuffer
> style: The name suggests to me that we are attaching a buffer TO the page h
I used the first one.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc@784
PS9, Line 784: batch->MarkFlushResources(); // TODO: remove
> This seems to be called at the same conditions as line 805. If this is inte
Oops, removed now.
--
To view, visit http://gerrit.cloudera.org:8080/11007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5
Gerrit-Change-Number: 11007
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Sat, 28 Jul 2018 03:14:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 10: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/96/ Running initial code review checks. 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/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 28 Jul 2018 03:14:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#10). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 392 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/10 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 10 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 14: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2873/ -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 28 Jul 2018 02:49:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4690: [DOCS] More content for CONV()
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11075 ) Change subject: IMPALA-4690: [DOCS] More content for CONV() .. Patch Set 1: (8 comments) Thanks for taking on this ticket! http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml File docs/topics/impala_math_functions.xml: http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@198 PS1, Line 198: num Calling the string parameter "num" is confusing. A few times below there are references like "if num is negative", but strings cannot be negative. Maybe give the parameters different names or just break up the two conv functions and describe them separately. http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@215 PS1, Line 215: null nit: can you capitalize "null" the same way each time? Either way is fine. http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@226 PS1, Line 226: between Maybe "more than -2 and less than 2" or just "-1, 0, or 1"? http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@231 PS1, Line 231: and from_base is negativ Can you explain more how a negative base is interpreted? https://en.wikipedia.org/wiki/Negative_base says you can represent positive numbers in a negative base. An example or two with a negative base would be useful. http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@231 PS1, Line 231: num elide http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@234 PS1, Line 234: if signed If *what* is signed? http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@234 PS1, Line 234: 18446744073709551615 Can you say what this number is? I assume it's 2^64 - 1. http://gerrit.cloudera.org:8080/#/c/11075/1/docs/topics/impala_math_functions.xml@251 PS1, Line 251: CAST() the return value Only works if to_base is 10, right? -- To view, visit http://gerrit.cloudera.org:8080/11075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 Gerrit-Change-Number: 11075 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Sat, 28 Jul 2018 02:28:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/95/ : 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/10908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Sat, 28 Jul 2018 01:01:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4690: [DOCS] More content for CONV()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11075 ) Change subject: IMPALA-4690: [DOCS] More content for CONV() .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/16/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 Gerrit-Change-Number: 11075 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Sat, 28 Jul 2018 00:52:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4690: [DOCS] More content for CONV()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11075 ) Change subject: IMPALA-4690: [DOCS] More content for CONV() .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/16/ 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/11075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 Gerrit-Change-Number: 11075 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 28 Jul 2018 00:35:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4690: [DOCS] More content for CONV()
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11075 Change subject: IMPALA-4690: [DOCS] More content for CONV() .. IMPALA-4690: [DOCS] More content for CONV() Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 --- M docs/topics/impala_math_functions.xml 1 file changed, 49 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/11075/1 -- To view, visit http://gerrit.cloudera.org:8080/11075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 Gerrit-Change-Number: 11075 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. Patch Set 11: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/95/ Running initial code review checks. 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/10908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Sat, 28 Jul 2018 00:21:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 108 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/11 -- To view, visit http://gerrit.cloudera.org:8080/10908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/94/ : 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/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 28 Jul 2018 00:19:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7170: Update data generator.py for Hadoop 3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11041 ) Change subject: IMPALA-7170: Update data_generator.py for Hadoop 3 .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2872/ -- To view, visit http://gerrit.cloudera.org:8080/11041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47b7d663174dbd38a5d9c98f1a88f0ebab726d5a Gerrit-Change-Number: 11041 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Sat, 28 Jul 2018 00:18:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10908 )
Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements
..
Patch Set 10:
(3 comments)
Sorry for the delay, I was busy this week.
http://gerrit.cloudera.org:8080/#/c/10908/7/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/10908/7/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138
PS7, Line 138: FeView
> I tried the changes discussed offline. However, the InlineViewRef objects a
Makes sense. So can we call this collectIlineViews(..views) or something since
we are not collecting ViewRefs anyway?
http://gerrit.cloudera.org:8080/#/c/10908/8/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/10908/8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138
PS8, Line 138: public abstract void collectInlineViewRefs(Set
inlineViewRefs);
> Done.
Let's keep both the versions for completeness. Looks like you replaced the
earlier one.
http://gerrit.cloudera.org:8080/#/c/10908/10/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/10908/10/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1092
PS10, Line 1092: if (withClause_ != null) {
: List withClauseViews =
withClause_.getViews();
: for (FeView withView : withClauseViews) {
: inlineViewRefs.add(withView);
:
withView.getQueryStmt().collectInlineViewRefs(inlineViewRefs);
: }
: }
Should this be moved to the QueryStmt class? See implementation of
collectTableRefs() for example.
--
To view, visit http://gerrit.cloudera.org:8080/10908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
Gerrit-Change-Number: 10908
Gerrit-PatchSet: 10
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Sat, 28 Jul 2018 00:06:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 14: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/93/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 28 Jul 2018 00:00:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10971 ) Change subject: IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog .. Patch Set 5: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10971/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10971/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@197 PS5, Line 197: public static Modifier 'public' is redundant for interface fields Modifier 'static' is redundant for interface fields -- To view, visit http://gerrit.cloudera.org:8080/10971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 Gerrit-Change-Number: 10971 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 23:55:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10998 )
Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX
metrics
..
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h
File be/src/util/default-path-handlers.h:
http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h@34
PS4, Line 34: MetricGroup* metric_group = NULL);
> why did you make the jmx path optional here? Is this because the statestore
Yea, its a common code path for all of them. Looks like getJNIEnv() creates a
JVM if one doesn't exist. I cleaned this up to add a flag in JniUtil which
tells if a JVM exists.
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java
File fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java:
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@71
PS4, Line 71: public class JMXJsonUtil {
> I didn't review this file particularly since it's just cross-ported. Any pa
Nothing much. Looks straightforward. I commented a couple of refactors that I
did incase you want to diff with the upstream version
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@79
PS4, Line 79: public static String getJMXJson() {
Removed a bunch of servlet code.
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java#L170
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@89
PS4, Line 89:
Removed the 'qry' functionality.
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/jmx/JMXJsonServlet.java#L206
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@198
PS4, Line 198: trace
Moved this from upstream debug() to trace() since the default logging in Impala
fe is debug(). This was firing for a few attributes and was dumping a bunch of
stacks and looks like for the same reason it was made debug in the Hadoop code.
https://github.com/apache/hadoop/commit/2d1406e9e7b75a833f79c0159031dae2ba3e6134
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@42
PS4, Line 42: public class JvmPauseMonitor {
> same here, any changes worth noting to take a look at?
Commented the refactoring I did. Nothing major.
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@42
PS4, Line 42: {
Removed dependency on AbstractService
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@69
PS4, Line 69: private JvmPauseMonitor() {
: this(INFO_THRESHOLD_MS, WARN_THRESHOLD_MS);
: }
Not relying on Hadoop Conf objects.
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@160
PS4, Line 160: Stopwatch
Uses Stopwatch from Guava.
--
To view, visit http://gerrit.cloudera.org:8080/10998
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341
Gerrit-Change-Number: 10998
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 27 Jul 2018 23:46:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Hello Tianyi Wang, Todd Lipcon, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10998 to look at the new patch set (#5). Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics Pause monitor: = This commit adds a stripped down version of Hadoop's JvmPauseMonitor class (https://bit.ly/2O6qSwm) . The core implementaion is borrowed from hadoop-common project and the hadoop dependencies are removed. - Removed dependency on AbstractService. - Not relying on Hadoop's Configuration object for reading confs. - Switched to Guava's implementation of Stopwatch. This utility class can detect both GC/non-GC pauses. In case of GC pauses, the GC metrics during the pause period are logged. Sample Output: = Detected pause in JVM or host machine (eg GC): pause of approximately 2356ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2241ms GC pool 'PS Scavenge' had collection(s): count=3 time=352ms Detected pause in JVM or host machine (eg GC): pause of approximately 1964ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2082ms GC pool 'PS Scavenge' had collection(s): count=1 time=251ms Detected pause in JVM or host machine (eg GC): pause of approximately 2120ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2454ms Detected pause in JVM or host machine (eg GC): pause of approximately 2238ms GC pool 'PS MarkSweep' had collection(s): count=5 time=13464ms Detected pause in JVM or host machine (eg GC): pause of approximately 2233ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2733ms JMX Metrics: JMX metrics are not emmitted for Impala and Catalog JVMs at the web end point /jmx. - Impalad: http(s)://:25000/jmx - Catalogd: http(s)://:25020/jmx A text only version (without HTML formatting) of the JMX metrics can be obtained by passing 'raw' parameter to the /jmx endpoint. For ex: http://:25000/jmx?raw This is useful for downstream tools that want to consume and process this output. Misc: Renamed JvmMetric -> JvmMemoryMetric to make the intent more clear. It doesn't relate to the functionality of the patch in anyway. Testing: === - Tested it manually with kill -SIGSTOP/-SIGCONT . Made sure that the non-GC JVM pauses are logged. - This class' functionality is tested manually by invoking it's main() - Injected a memory leak into the Catalog server code and made sure the GC is detected. Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 --- M be/src/common/init.cc M be/src/util/default-path-handlers.cc M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M be/src/util/metrics-test.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/common/JniUtil.java A fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java A fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java 11 files changed, 633 insertions(+), 70 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/10998/5 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 5: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/94/ Running initial code review checks. 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/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 23:45:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7308. Support Avro tables in LocalCatalog
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10970 ) Change subject: IMPALA-7308. Support Avro tables in LocalCatalog .. Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/10970/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10970/5//COMMIT_MSG@39 PS5, Line 39: if an Avro partition is added to a non-Avro table, and that partition : has a schema that isn't compatible with the table's schema, an error : will occur on read. Can we test this yet? Or is the plan to test existing and local catalog together after IMPALA-7309? http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1558 PS5, Line 1558: ; Why don't we return here? Is there any need to reconcile a inferred schema? http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/main/java/org/apache/impala/util/AvroSchemaUtils.java File fe/src/main/java/org/apache/impala/util/AvroSchemaUtils.java: http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/main/java/org/apache/impala/util/AvroSchemaUtils.java@47 PS5, Line 47: HdfsTable AvroSchemaUtils http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java File fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java: http://gerrit.cloudera.org:8080/#/c/10970/5/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@239 PS5, Line 239: // TODO(todd): do we have any tables which are mixed format? alltypesmixedformat: https://github.com/apache/impala/blob/b5608264b4552e44eb73ded1e232a8775c3dba6b/testdata/bin/load-dependent-tables.sql#L62 -- To view, visit http://gerrit.cloudera.org:8080/10970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 Gerrit-Change-Number: 10970 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 23:30:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 14: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/93/ Running initial code review checks. 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/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 27 Jul 2018 23:25:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2873/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 27 Jul 2018 23:25:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 14: Code-Review+1 rebase -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 27 Jul 2018 23:25:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Hello Michael Ho, Pranay Singh, Quanlong Huang, Lars Volker, Fredy Wijaya, Todd
Lipcon, Bikramjeet Vig, Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10744
to look at the new patch set (#14).
Change subject: IMPALA-1760: Implement shutdown command
..
IMPALA-1760: Implement shutdown command
This allows graceful shutdown of executors and partially graceful
shutdown of coordinators (new operations fail, old operations can
continue).
Details:
* In order to allow future admin commands, this is implemented with
function-like syntax and does not add any reserved words.
* ALL privilege is required on the server
* The coordinator impalad that the client is connected to can be shut
down directly with ":shutdown()".
* Remote shutdown of another impalad is supported, e.g. with
":shutdown('hostname')", so that non-coordinators can be shut down
and for the convenience of the client, which does not have to
connect to the specific impalad. There is no assumption that the
other impalad is registered in the statestore; just that the
coordinator can connect to the other daemon's thrift endpoint.
This simplifies things and allows shutdown in various important
cases, e.g. statestore down.
* The shutdown time limit can be overridden to force a quicker or
slower shutdown by specifying a deadline in seconds after the
statement is executed.
* If shutting down, a banner is shown on the root debug page.
Workflow:
1. (if a coordinator) clients are prevented from submitting
queries to this coordinator via some out-of-band mechanism,
e.g. load balancer
2. the shutdown process is started via ":shutdown()"
3. a bit is set in the statestore and propagated to coordinators,
which stop scheduling fragment instances on this daemon
(if an executor).
4. the quiesce period (which is ideally set to the AC queueing
delay plus some additional leeway) expires
5. once the daemon is drained (i.e. no fragments, no registered
queries), it shuts itself down.
6. If the daemon does not drain (e.g. rogue clients, long-running
queries), after a longer timeout it will shut down anyway.
What this does:
* Executors can be shut down without causing a service-wide outage
* Shutting down an executor will not disrupt any short-running queries
and will wait for long-running queries up to a threshold.
* Coordinators can be shut down without query failures only if
there is an out-of-band mechanism to prevent submission of more
queries to the shut down coordinator. If queries are submitted to
a coordinator after shutdown has started, they will fail.
* Long running queries or other issues (e.g. stuck fragments) will
slow down but not prevent eventual shutdown.
Limitations:
* The quiesce period needs to be configured to be greater than the
latency of statestore updates + scheduling + admission +
coordinator startup. Otherwise a coordinator may send a
fragment instance to the shutting down impalad. (We could
automate this configuration as a follow-on)
* The quiesce period means a minimum latency for shutdown,
even if the cluster is idle.
* We depend on the statestore detecting the process going down
if queries are still running on that backend when the timeout
expires. This may still be subject to existing problems,
e.g. IMPALA-2990.
Tests:
* Added parser, analysis and authorization tests.
* End-to-end test of shutting down impalads.
* End-to-end test of shutting down then restarting an executor while
queries are running.
* End-to-end test of shutting down a coordinator
- New queries cannot be started on coord, existing queries continue to run
- Exercises various Beeswax and HS2 operations.
Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0
---
M be/src/runtime/backend-client.h
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/scheduling/scheduler.cc
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/testutil/fault-injection-util.h
M be/src/util/default-path-handlers.cc
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/StatestoreService.thrift
M common/thrift/Types.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11007 )
Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
..
Patch Set 9:
(5 comments)
The new test seems mind-bending this late at night. I will give it another run
on Monday. I have some nits and style comments till then.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@863
PS9, Line 863: void SimpleTupleStreamTest::TestFlushResourcesReadWrite(bool
pin_stream, bool attach_on_read) {
nit: long line
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@913
PS9, Line 913: for (int j = 0; j < out_batch->num_rows(); ++j) {
style: Level 5 loop nesting seems a bit too deep to me - can you move some of
the inner loops into separate functions? For example the verifying logic from
line 908 to 918 seems a good candidate to me. This is definitely subjective, so
feel free to keep it as it is.
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream-test.cc@916
PS9, Line 916:
*static_cast(out_batch->GetRow(j)->GetTuple(0)->GetSlot(slot_offset)));
nit: long line
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.h@395
PS9, Line 395: AttachBuffer
style: The name suggests to me that we are attaching a buffer TO the page here.
Some names that came to my mind are "AttachBufferToBatch", "ExtractBuffer" or
"TakeBuffer".
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:
http://gerrit.cloudera.org:8080/#/c/11007/9/be/src/runtime/buffered-tuple-stream.cc@784
PS9, Line 784: batch->MarkFlushResources(); // TODO: remove
This seems to be called at the same conditions as line 805. If this is
intentional, can you explain why?
--
To view, visit http://gerrit.cloudera.org:8080/11007
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5
Gerrit-Change-Number: 11007
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 23:03:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/10998 )
Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX
metrics
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h
File be/src/util/default-path-handlers.h:
http://gerrit.cloudera.org:8080/#/c/10998/4/be/src/util/default-path-handlers.h@34
PS4, Line 34: MetricGroup* metric_group = NULL, bool init_jmx = false);
why did you make the jmx path optional here? Is this because the statestore
doesn't use Java or something? In that case could we instead just use getJNIEnv
or something else to determine it rather than adding this param?
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java
File fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java:
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java@71
PS4, Line 71: public class JMXJsonUtil {
I didn't review this file particularly since it's just cross-ported. Any
particular bits that merit a special look that you had to change?
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java
File fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java:
http://gerrit.cloudera.org:8080/#/c/10998/4/fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java@42
PS4, Line 42: public class JvmPauseMonitor {
same here, any changes worth noting to take a look at?
--
To view, visit http://gerrit.cloudera.org:8080/10998
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341
Gerrit-Change-Number: 10998
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 27 Jul 2018 22:58:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7324: remove MarkNeedsDeepCopy() from sorter
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11048 ) Change subject: IMPALA-7324: remove MarkNeedsDeepCopy() from sorter .. IMPALA-7324: remove MarkNeedsDeepCopy() from sorter Testing: Ran exhaustive tests. Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Reviewed-on: http://gerrit.cloudera.org:8080/11048 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/row-batch.cc M be/src/runtime/row-batch.h M be/src/runtime/sorted-run-merger.cc M be/src/runtime/sorted-run-merger.h M be/src/runtime/sorter.cc 5 files changed, 52 insertions(+), 64 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Gerrit-Change-Number: 11048 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7324: remove MarkNeedsDeepCopy() from sorter
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11048 ) Change subject: IMPALA-7324: remove MarkNeedsDeepCopy() from sorter .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Gerrit-Change-Number: 11048 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 22:34:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3330: [DOCS] TRANSLATE function updated
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11074 ) Change subject: IMPALA-3330: [DOCS] TRANSLATE function updated .. Patch Set 1: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/15/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica33ecbb7118e3034f95c5705eed19d169dc16cb Gerrit-Change-Number: 11074 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Comment-Date: Fri, 27 Jul 2018 22:33:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3330: [DOCS] TRANSLATE function updated
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11074 ) Change subject: IMPALA-3330: [DOCS] TRANSLATE function updated .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/15/ 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/11074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica33ecbb7118e3034f95c5705eed19d169dc16cb Gerrit-Change-Number: 11074 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 27 Jul 2018 22:22:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3330: [DOCS] TRANSLATE function updated
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11074 Change subject: IMPALA-3330: [DOCS] TRANSLATE function updated .. IMPALA-3330: [DOCS] TRANSLATE function updated - Better description - Examples Change-Id: Ica33ecbb7118e3034f95c5705eed19d169dc16cb --- M docs/topics/impala_string_functions.xml 1 file changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/11074/1 -- To view, visit http://gerrit.cloudera.org:8080/11074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ica33ecbb7118e3034f95c5705eed19d169dc16cb Gerrit-Change-Number: 11074 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/92/ : 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/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 22:21:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/91/ : 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/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 8 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Fri, 27 Jul 2018 22:12:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Code-Review+1 (2 comments) Will let Attila take a look too http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9 PS2, Line 9: This change adds a new query option "timezone" which > I am unsure about this - Linux uses /etc/timezone and $TZ for the same conc Yeah I kind of like the shorter name "timezone" upon further reflection. http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/2/tests/shell/test_shell_commandline.py@706 PS2, Line 706: assert actual_time_s <= time_limit_s, ( Can you also add a test to tests/metadata/test_set.py that exercises the server-side 'set' command? -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 22:11:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 4: Screenshots: Raw JMX metrics: https://pasteboard.co/HwsbaAD.png Formatted : https://pasteboard.co/HwsbvyI.png -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 21:56:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/10998/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10998/3//COMMIT_MSG@42 PS3, Line 42: : JMX Metrics: : > Personally I wouldn't find those aggregate counts super useful (doesn't giv Ok, removed them. I think we can do the histogram of GCs etc as a follow on patch. I feel the patch in the current form is pretty useful and makes the metrics available to the thirdparty tools to consume. http://gerrit.cloudera.org:8080/#/c/10998/3/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/10998/3/common/thrift/Frontend.thrift@830 PS3, Line 830: > if you don't take my other suggestion about exposing "/jmx" directly, it wo switched to /jmx -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 21:53:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Hello Tianyi Wang, Todd Lipcon, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10998 to look at the new patch set (#4). Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics Pause monitor: = This commit adds a stripped down version of Hadoop's JvmPauseMonitor class (https://bit.ly/2O6qSwm) . The core implementaion is borrowed from hadoop-common project and the hadoop dependencies are removed. - Removed dependency on AbstractService. - Not relying on Hadoop's Configuration object for reading confs. - Switched to Guava's implementation of Stopwatch. This utility class can detect both GC/non-GC pauses. In case of GC pauses, the GC metrics during the pause period are logged. Sample Output: = Detected pause in JVM or host machine (eg GC): pause of approximately 2356ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2241ms GC pool 'PS Scavenge' had collection(s): count=3 time=352ms Detected pause in JVM or host machine (eg GC): pause of approximately 1964ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2082ms GC pool 'PS Scavenge' had collection(s): count=1 time=251ms Detected pause in JVM or host machine (eg GC): pause of approximately 2120ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2454ms Detected pause in JVM or host machine (eg GC): pause of approximately 2238ms GC pool 'PS MarkSweep' had collection(s): count=5 time=13464ms Detected pause in JVM or host machine (eg GC): pause of approximately 2233ms GC pool 'PS MarkSweep' had collection(s): count=1 time=2733ms JMX Metrics: JMX metrics are not emmitted for Impala and Catalog JVMs at the web end point /jmx. - Impalad: http(s)://:25000/jmx - Catalogd: http(s)://:25020/jmx A text only version (without HTML formatting) of the JMX metrics can be obtained by passing 'raw' parameter to the /jmx endpoint. For ex: http://:25000/jmx?raw This is useful for downstream tools that want to consume and process this output. Misc: Renamed JvmMetric -> JvmMemoryMetric to make the intent more clear. It doesn't relate to the functionality of the patch in anyway. Testing: === - Tested it manually with kill -SIGSTOP/-SIGCONT . Made sure that the non-GC JVM pauses are logged. - This class' functionality is tested manually by invoking it's main() - Injected a memory leak into the Catalog server code and made sure the GC is detected. Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/runtime/exec-env.cc M be/src/util/default-path-handlers.cc M be/src/util/default-path-handlers.h M be/src/util/jni-util.cc M be/src/util/jni-util.h M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M be/src/util/metrics-test.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/common/JniUtil.java A fe/src/main/java/org/apache/impala/util/JMXJsonUtil.java A fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java 14 files changed, 629 insertions(+), 73 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/10998/4 -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility and expose JMX metrics .. Patch Set 4: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/92/ Running initial code review checks. 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/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 21:50:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h@308 PS6, Line 308: : /// Number of active fragment instances and coordinators for this query that may consume : /// resources for query execution (i.e. threads, memory) on the Impala daemon. : /// Query-wide execution resources for this query are released once this goes to zero. : AtomicInt32 exec_resource_refcnt_; : : /// Temporary files for this query (owned by obj_pool_). Non-null if spilling is : /// enabled. Set in Pr > Now that the patch has sunk in more, I think the state machine the query st Yes, this is correct. With IMPALA-4063, we will report errors earlier in the lifecycle first (i.e. PREPARING errors before EXECUTING errors). I think reasoning about errors earlier in the query lifecycle is easier than reasoning about errors hit first in terms of a timestamp. -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 8 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Fri, 27 Jul 2018 21:48:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. (
http://gerrit.cloudera.org:8080/10813 )
Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..
Patch Set 8:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/fragment-instance-state.cc@125
PS7, Line 125: DCHECK_
> Does DCHECK_EQ() not work ?
Done
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h@67
PS7, Line 67:
: /// Eg: We transition from the PREPARING state to the EXECUTING
state only if *all* the
: /// underlying fragment instances have finished Prepare().
> This is not true for PREPARING phase, right ? Seems easier to separately de
Done
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h@70
PS7, Line 70: ansitioni
> PREPARING ?
Done
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h@71
PS7, Line 71: y fragment instance hits an error or cancella
> This is not necessarily true, right ? In PREPARING state, it only means at
You're right. I changed the wording now.
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h@264
PS7, Line 264:
> May want to document the thread safety of this variable. I suppose it's onl
Yup, done.
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.h@287
PS7, Line 287: /// Protected by 'status_lock_'.
> Please consider documenting the thread safety of this variable too.
Done
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.cc@230
PS7, Line 230: BackendExecState::EXECUTING : BackendExecState::FINISHED;
: }
> nit: one line
Done
http://gerrit.cloudera.org:8080/#/c/10813/7/be/src/runtime/query-state.cc@479
PS7, Line 479: {
: unique_lock l(status_lock_);
: query_status_ = Status::CANCELLED;
: }
> Should this happen after line 483 below ?
No, it makes sense to preemptively set this since a Cancel() is a query wide
operation and not specific to a fragment instance.
The ErrorDuring*() functions don't update the 'query_status_' if it's not OK,
so that's not an issue.
http://gerrit.cloudera.org:8080/#/c/10813/8/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/8/be/src/runtime/query-state.cc@390
PS8, Line 390: // Fragment instance successfully started
: fis_map_.emplace(fis->instance_id(), fis);
I had to move this up here since there's a race with the coordinator between
L141 and L156 in the coordinator.cc file
--
To view, visit http://gerrit.cloudera.org:8080/10813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 8
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Fri, 27 Jul 2018 21:42:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 8: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/91/ Running initial code review checks. 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/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 8 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Fri, 27 Jul 2018 21:41:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Hello Michael Ho, Joe McDonnell, Bikramjeet Vig, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10813 to look at the new patch set (#8). Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. IMPALA-7163: Implement a state machine for the QueryState class This patch adds a state machine for the QueryState class. The motivation behind this patch is to make the query lifecycle from the point of view of an executor much easier to reason about and this patch is key for a follow on patch for IMPALA-2990 where the status reporting will be per-query rather than per-fragment-instance. Currently, the state machine provides no other purpose, and it will mostly be used for IMPALA-2990. We introduce 5 possible states for the QueryState which include 3 terminal states (FINISHED, CANCELLED and ERROR) and 2 non-terminal states (PREPARING, EXECUTING). The transition from one state to the next is always handled by a single thread which is also the QueryState thread. This thread will additionally bear the purpose of sending periodic updates after IMPALA-4063, which is the primary reason behind having only this thread modify the state of the query. Counting barriers are introduced to keep a count of how many fragment instances have finished Preparing and Executing. These barriers also block until all the fragment instances have finished a respective state. The fragment instances update the query wide query status if an error is hit and unblocks the barrier if it is in the EXECUTING state. The PREPARING state blocks regardless of whether a fragment instance hit an error or not, until all the fragment instances have completed successfully or unsuccessfully, to maintain the invariant that fragment instances cannot be cancelled until the entire QueryState has finished PREPARING. The status reporting protocol has not been changed and remains exactly as it was. Testing: Ran 'core' and 'exhaustive' tests. TODO: Run stress tests. Future related work: 1) IMPALA-2990: Make status reporting per-query. 2) Try to logically align the FIS states with the QueryState states. 3) Consider mirroring the QueryState state machine to CoordinatorBackendState Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe --- M be/src/runtime/coordinator.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h 5 files changed, 233 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10813/8 -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 8 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/90/ : 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/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:30:59 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 ) Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries .. Patch Set 4: (8 comments) http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@302 PS4, Line 302: it nit: this lock http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@309 PS4, Line 309: releases the memory associated with : /// filter_routing_table_ nit: alters the filter_routing_table_ http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@311 PS4, Line 311: boost::shared_mutex We have to ensure that this is a 'writer preferred' shared mutex. It turns out that boost doesn't have an option to specify a preference, and uses a "fair" algorithm instead to decide which waiter to schedule next, so I guess this is fine. http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@79 PS4, Line 79: exec_state_.Load(), ExecState::EXECUTING not your change, but replace with: !IsExecuting() http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@397 PS4, Line 397: lock_guard l(filter_update_lock_); Why not just get exclusive access to 'filter_lock_' here? http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@454 PS4, Line 454: exec_state_.Load() != ExecState::EXECUTING not your change, but replace with: !IsExecuting() http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@714 PS4, Line 714: exec_state_.Load() == ExecState::EXECUTING not your change, but replace with: IsExecuting() http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@871 PS4, Line 871: if (!IsExecuting()) return; Instead of doing this here, I would say you can replace the IsDone() check inside CoordinatorBackendState::PublishFilter() with this IsExecuting() check. CoordinatorBackendState has a reverse reference to the Coordinator object. -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 4 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:17:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11068 ) Change subject: IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/14/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/11068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f Gerrit-Change-Number: 11068 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zsombor Fedor Gerrit-Comment-Date: Fri, 27 Jul 2018 21:15:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:10:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7170: Update data generator.py for Hadoop 3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11041 ) Change subject: IMPALA-7170: Update data_generator.py for Hadoop 3 .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2872/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47b7d663174dbd38a5d9c98f1a88f0ebab726d5a Gerrit-Change-Number: 11041 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:06:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 4 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 21:01:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. IMPALA-5031: Fix undefined behavior: right-shifting too far In expr.shift, the C++ standard says of right shifts: The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand. In HdfsAvroScannerTest.DecimalTest, this is triggered, and the interesting part of the backtrace is: exec/hdfs-avro-scanner-ir.cc:272:18: runtime error: shift exponent 32 is too large for 32-bit type 'int32_t' (aka 'int') #0 0x1786f65 in HdfsAvroScanner::ReadAvroDecimal(int, unsigned char**, unsigned char*, bool, void*, MemPool*) exec/hdfs-avro-scanner-ir.cc:272:18 #1 0x1617778 in void HdfsAvroScannerTest::TestReadAvroType, bool (HdfsAvroScanner::*)(int, unsigned char**, unsigned char*, bool, void*, MemPool*), unsigned long>(bool (HdfsAvroScanner::*)(int, unsigned char**, unsigned char*, bool, void*, MemPool*), unsigned long, unsigned char*, long, DecimalValue, int, TErrorCode::type) exec/hdfs-avro-scanner-test.cc:88:20 #2 0x1605705 in void HdfsAvroScannerTest::TestReadAvroDecimal(unsigned char*, long, DecimalValue, int, TErrorCode::type) exec/hdfs-avro-scanner-test.cc:184:5 Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Reviewed-on: http://gerrit.cloudera.org:8080/11047 Reviewed-by: Jim Apple Tested-by: Impala Public Jenkins --- M be/src/exec/hdfs-avro-scanner-ir.cc 1 file changed, 10 insertions(+), 0 deletions(-) Approvals: Jim Apple: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 5 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11068 ) Change subject: IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/14/ 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/11068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f Gerrit-Change-Number: 11068 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zsombor Fedor Gerrit-Comment-Date: Fri, 27 Jul 2018 21:00:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups
Hello Fredy Wijaya, Zsombor Fedor, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11068 to look at the new patch set (#3). Change subject: IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups .. IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups Added clarifications about delegation. Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f --- M docs/topics/impala_delegation.xml 1 file changed, 217 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/11068/3 -- To view, visit http://gerrit.cloudera.org:8080/11068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f Gerrit-Change-Number: 11068 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zsombor Fedor
[Impala-ASF-CR] IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11068 ) Change subject: IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml File docs/topics/impala_delegation.xml: http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@77 PS2, Line 77: 'user1=user2' > It maybe better to call this authenticated_user instead of user1 and also d Done http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@123 PS2, Line 123: group1 > This is wrong. The left-hand side is still a user. The client still sends t Done http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@162 PS2, Line 162: The delegation process works as follows: > Please also explain the user delegation with --authorized_proxy_group_confi Done -- To view, visit http://gerrit.cloudera.org:8080/11068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f Gerrit-Change-Number: 11068 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zsombor Fedor Gerrit-Comment-Date: Fri, 27 Jul 2018 20:59:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 9: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/90/ Running initial code review checks. 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/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 20:56:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11007 ) Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc File be/src/runtime/buffered-tuple-stream.cc: http://gerrit.cloudera.org:8080/#/c/11007/8/be/src/runtime/buffered-tuple-stream.cc@763 PS8, Line 763: has_read_write_page > I was able to hit a DCHECK in NextReadPage() with a new test, will work on I added a regression test that can trigger the two bugs you identified and fixed them. -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 20:56:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS
Hello Thomas Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11007 to look at the new patch set (#9). Change subject: IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS .. IMPALA-7333: remove MarkNeedsDeepCopy() in agg and BTS This takes advantage of work (e.g. IMPALA-3200, IMPALA-5844) to remove a couple of uses of the API. Testing: Ran core, ASAN and exhaustive builds. Added unit tests to directly test the attaching behaviour. Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 --- M be/src/exec/grouping-aggregator.cc M be/src/runtime/buffered-tuple-stream-test.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/buffered-tuple-stream.h M be/src/runtime/row-batch.h 5 files changed, 383 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/11007/9 -- To view, visit http://gerrit.cloudera.org:8080/11007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I91ac53bacc00df4726c015a30ba5a2026aa4b5f5 Gerrit-Change-Number: 11007 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7170: Update data generator.py for Hadoop 3
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11041 ) Change subject: IMPALA-7170: Update data_generator.py for Hadoop 3 .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47b7d663174dbd38a5d9c98f1a88f0ebab726d5a Gerrit-Change-Number: 11041 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 20:45:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add recent heartbeat timestamp into Statestore metric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/89/ : 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/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Fri, 27 Jul 2018 20:03:24 + Gerrit-HasComments: No
[Impala-ASF-CR] PREVIEW: IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: PREVIEW: IMPALA-110: Support for multiple DISTINCT .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/87/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 19:52:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 3): Add multiple DISTINCT support to query generator
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11073 ) Change subject: IMPALA-110 (part 3): Add multiple DISTINCT support to query generator .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/88/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11073 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a3f14655719ade7b2f6471c561dba4007fd46fa Gerrit-Change-Number: 11073 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 19:44:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7324: remove MarkNeedsDeepCopy() from sorter
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11048 ) Change subject: IMPALA-7324: remove MarkNeedsDeepCopy() from sorter .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2871/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Gerrit-Change-Number: 11048 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 19:27:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7324: remove MarkNeedsDeepCopy() from sorter
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11048 ) Change subject: IMPALA-7324: remove MarkNeedsDeepCopy() from sorter .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Gerrit-Change-Number: 11048 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 19:27:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6857: Add Jvm pause/GC Monitor utility
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10998 ) Change subject: IMPALA-6857: Add Jvm pause/GC Monitor utility .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10998/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10998/3//COMMIT_MSG@42 PS3, Line 42: These metrics are exposed on the daemon's web UI in /memz page under : section "JVM Garbage Collection metrics". They can also be consumed : by tools from the /metrics endpoint. > I have a working patch for /jmx end point. Before I send it out for review, Personally I wouldn't find those aggregate counts super useful (doesn't give a sense of whether the server is *currently* having GC issues) but I dont think there's harm in including them. -- To view, visit http://gerrit.cloudera.org:8080/10998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d897b7e063846ad6d8f88243e2c04264da0341 Gerrit-Change-Number: 10998 Gerrit-PatchSet: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 19:22:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7324: remove MarkNeedsDeepCopy() from sorter
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11048 ) Change subject: IMPALA-7324: remove MarkNeedsDeepCopy() from sorter .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If86f038f1f6ca81ad5c9d40af1b7ea6115144ffc Gerrit-Change-Number: 11048 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 19:19:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add recent heartbeat timestamp into Statestore metric
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h@417 PS1, Line 417: Sets > nit: typo Done http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h@442 PS1, Line 442: This timestamp is updated : /// every "statestore_heartbeat_log_frequency_seconds" seco > needs more explanation. how about instead: Done http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc@390 PS1, Line 390: _h > FLAGS_statestore_heartbeat_log_frequency_seconds I have removed this comment and instead modified the DCHECK. http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc@389 PS1, Line 389: DCHECK(timestamp_seconds : > (recent_heartbeat_timestamp_ + FLAGS_statestore_heartbeat_log_frequency_seconds)); : recent_heartbeat_timestamp_ = timestamp_seconds; > maybe instead of the comment, a dcheck for the following should be self exp Done -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Fri, 27 Jul 2018 19:18:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add recent heartbeat timestamp into Statestore metric
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/89/ Running initial code review checks. 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/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Fri, 27 Jul 2018 19:18:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add recent heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric .. IMPALA-6644: Add recent heartbeat timestamp into Statestore metric This patch collects the recent heartbeat timestamp of a subscriber in 60 second intervals. It publishes this timsetamp to the /subscribers page of the statestore and also adds it to the log. Testing: Manually inspected the Web UI and statestore logs to verify that the recent heartbeat timestamp for each subscriber is updated periodically. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M www/statestore_subscribers.tmpl 3 files changed, 37 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/2 -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] PREVIEW: IMPALA-110: Support for multiple DISTINCT
Hello Michael Ho, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10771 to look at the new patch set (#2). Change subject: PREVIEW: IMPALA-110: Support for multiple DISTINCT .. PREVIEW: IMPALA-110: Support for multiple DISTINCT This patch adds support for having multiple aggregate functions in a single SELECT block that use DISTINCT over different sets of columns. Planner design: - The existing tree-based plan shape with a two-phased aggregation is maintained. - Existing plans are not changed. - Aggregates are grouped into 'aggregation classes' based on their expressions in the distinct portion which may be empty for non-distinct aggregates. - The aggregation framework is generalized to simultaneously process multiple aggregation classes within the tree-based plan. This process splits the results of different aggregation classes into separate rows, so a final aggregation is needed to transpose the results into the desired form. - Main challenge: Each aggregation class consumes and produces different tuples, so conceptually a union-type of tuples flows through the runtime. The tuple union is represented by a TupleRow with one tuple per aggregation class. Only one tuple in such a TupleRow is non-NULL. - Backend exec nodes in the aggregation plan will be aware of this tuple-union either explicitly in their implementation or by relying on expressions that distinguish the aggregation classes. - To distinguish the aggregation classes, e.g. in hash exchanges, CASE expressions are crafted to hash/group on the appropriate slots. Deferred FE work: - Beautify/condense the long CASE exprs - Push applicable conjuncts into individual aggregators before the transposition step - Added a few testing TODOs to reduce the size of this patch - Decide whether we want to change existing plans to the new model Execution design: - Previous patches separated out aggregation logic from the exec node into Aggregators. This is extended to support multiple Aggregators per node, with different grouping and aggregating functions. - There is a fast path for aggregations with only one aggregator, which leaves the execution essentially unchanged from before. - When there are multiple aggregators, the first aggregation node in the plan replicates its input to each aggregator. The output of this step is rows where only a single tuple is non-null, corresponding to the aggregator that produced the row. - A new expr is introduced, ValidTupleId, which takes one of these rows and returns which tuple is non-null. - For additional aggregation nodes, the input is split apart into 'mini-batches' according to which aggregator the row corresponds to. Testing: - Added analyzer and planner tests - Added end-to-end queries tests - Ran hdfs/core tests Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 --- M be/src/exec/CMakeLists.txt A be/src/exec/aggregation-node-base.cc A be/src/exec/aggregation-node-base.h M be/src/exec/aggregation-node.cc M be/src/exec/aggregation-node.h M be/src/exec/aggregator.cc M be/src/exec/aggregator.h M be/src/exec/exec-node.cc M be/src/exec/grouping-aggregator-ir.cc M be/src/exec/grouping-aggregator.cc M be/src/exec/grouping-aggregator.h M be/src/exec/non-grouping-aggregator.cc M be/src/exec/non-grouping-aggregator.h M be/src/exec/streaming-aggregation-node.cc M be/src/exec/streaming-aggregation-node.h M be/src/exprs/CMakeLists.txt M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/aggregate-functions.h M be/src/exprs/scalar-expr.cc A be/src/exprs/valid-tuple-id.cc A be/src/exprs/valid-tuple-id.h M common/thrift/Exprs.thrift M common/thrift/PlanNodes.thrift M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java A fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java A fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java M fe/src/main/java/org/apache/impala/planner/AggregationNode.java M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java M fe/src/main/java/org/apache/impala/planner/UnionNode.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M
[Impala-ASF-CR] IMPALA-110 (part 3): Add multiple DISTINCT support to query generator
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11073 ) Change subject: IMPALA-110 (part 3): Add multiple DISTINCT support to query generator .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/88/ Running initial code review checks. 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/11073 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a3f14655719ade7b2f6471c561dba4007fd46fa Gerrit-Change-Number: 11073 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 27 Jul 2018 19:12:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110 (part 3): Add multiple DISTINCT support to query generator
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11073 Change subject: IMPALA-110 (part 3): Add multiple DISTINCT support to query generator .. IMPALA-110 (part 3): Add multiple DISTINCT support to query generator Previously, Impala was only able to support DISTINCT in aggregate functions over a single expr per SELECT list. IMPALA-110 removes this restriction. This patch eliminates code in query_generator.py that grouped exprs for aggregate functions in order to pick a single to make DISTINCT, and instead simply iterates over all agg functions and makes each one DISTINCT with a configurable probability. Testing: - Ran the query generator overnight with no problems (except the usual false positives). Change-Id: I4a3f14655719ade7b2f6471c561dba4007fd46fa --- M tests/comparison/query_generator.py 1 file changed, 10 insertions(+), 51 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/11073/1 -- To view, visit http://gerrit.cloudera.org:8080/11073 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4a3f14655719ade7b2f6471c561dba4007fd46fa Gerrit-Change-Number: 11073 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] PREVIEW: IMPALA-110: Support for multiple DISTINCT
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: PREVIEW: IMPALA-110: Support for multiple DISTINCT .. Patch Set 2: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/87/ Running initial code review checks. 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/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 19:12:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add recent heartbeat timestamp into Statestore metric
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h@417 PS1, Line 417: Set's nit: typo http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.h@442 PS1, Line 442: This timestamp must be within the : /// duration of statestore_heartbeat_log_frequency_seconds. needs more explanation. how about instead: " Is updated every "statestore_heartbeat_log_frequency_seconds" seconds with the last heartbeat. A timestamp much older than the update frequency implies an unresponsive subscriber. " http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc@390 PS1, Line 390: 60 FLAGS_statestore_heartbeat_log_frequency_seconds http://gerrit.cloudera.org:8080/#/c/11052/1/be/src/statestore/statestore.cc@389 PS1, Line 389: // Ensure that the timestamp to be updated is greater than the current value stored. : // We can't guarantee that the difference is exactly 60 seconds because we are storing : // the heartbeat timestamp in seconds while the hearbeats are sent maybe instead of the comment, a dcheck for the following should be self explanatory. timestamp_seconds > (recent_heartbeat_timestamp_ + FLAGS_statestore_heartbeat_log_frequency_seconds) -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 1 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Fri, 27 Jul 2018 19:00:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 13: This is awesome. I look forward to reviewing this later. -- To view, visit http://gerrit.cloudera.org:8080/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 18:09:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7203. Support UDFs in LocalCatalog
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11053 ) Change subject: IMPALA-7203. Support UDFs in LocalCatalog .. Patch Set 1: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/11053/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11053/1//COMMIT_MSG@1 PS1, Line 1: Parent: 0af4661c (IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded) In my understanding the transient UDF is a legacy feature and is not widely used. Is that correct? http://gerrit.cloudera.org:8080/#/c/11053/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11053/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@166 PS1, Line 166: MetaException No need to declare MetaException because it extends TException. There are many other instances of this redundancy. -- To view, visit http://gerrit.cloudera.org:8080/11053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed Gerrit-Change-Number: 11053 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 27 Jul 2018 18:03:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2870/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:54:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. Patch Set 4: Code-Review+2 Carry Tim's +2 -- To view, visit http://gerrit.cloudera.org:8080/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 4 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:51:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2869/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 4 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:51:56 + Gerrit-HasComments: No
[Impala-ASF-CR] Bump Kudu version to 5211897
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/86/ : 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/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:51:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/85/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:50:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@9 PS2, Line 9: This change adds a new query option "timezone" which > I wonder if something like local_timezone would be clearer? Or is that too I am unsure about this - Linux uses /etc/timezone and $TZ for the same concept. I am not against local_timezone - if everyone likes it then I can change the name in the next patch. http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@11 PS2, Line 11: The main goal is to simplify testing, but I think that > Can you briefly mention that this is meant to be a user-visible option rath Done http://gerrit.cloudera.org:8080/#/c/11064/2//COMMIT_MSG@24 PS2, Line 24: cannot be entered unquoted in some contexts. : : Currently the timezone has effect in the following cases: : -function now() > Is there any potential interaction with table properties or similar? I don' isAdjustedToUTC is included in the metadata in the Parquet file's footer - the reason for doing this is that Hive (and probably Sparc?) does a local->utc conversion when writing the timestamp, and Impala needs to convert it back to see the same results. Meanwhile Impala writes and reads it without any conversion (which is much faster). "to convert on not to convert" could be decided by the writer's version and the flag until now. Newer versions of Hive may write the timestamps in a way that will need no conversion by Impala to improve scan speed - as the same writer will write different timestamps depending on configuration, the writer's version will not be enough the decide, so this information has to be included in the metadata. Writing it to table property would not work because different applications may write the files of the same table. This logic is added in https://gerrit.cloudera.org/#/c/11057/1/be/src/exec/parquet-column-readers.cc / line 412. http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@687 PS2, Line 687: // Leading/trailing " and ' characters are stripped because the / character > The stripping might need a brief explanation Done http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689 PS2, Line 689: string timezone = value; > Missing " " before ( Done http://gerrit.cloudera.org:8080/#/c/11064/2/be/src/service/query-options.cc@689 PS2, Line 689: > nullptr Done http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/11064/2/common/thrift/ImpalaService.thrift@331 PS2, Line 331: The default is the > This is a little unclear because the default value is "". I guess this mean Thanks for spotting the inconsistency - it remained there from a different state of the patch. http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/2/tests/custom_cluster/test_local_tz_conversion.py@62 PS2, Line 62: > Can we combine this with test_utc_timestamp_functions() to avoid restarting Done -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:50:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11047 )
Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far
..
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc
File be/src/exec/hdfs-avro-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc@263
PS3, Line 263: if(LIKELY(slot_byte_size == 4 || slot_byte_size == 8 ||
slot_byte_size == 16)) {
> It doesn't look sort-of free to me - The Compiler Explorer reports every pa
Oh i guess it's replaced with a constant for codegen anyway
--
To view, visit http://gerrit.cloudera.org:8080/11047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8
Gerrit-Change-Number: 11047
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 17:36:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11068 ) Change subject: IMPALA-7195 IMPALA-7222: [DOCS] Impala delegation with groups .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml File docs/topics/impala_delegation.xml: http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@77 PS2, Line 77: 'user1=user2' It maybe better to call this authenticated_user instead of user1 and also delegated_user instead of user2 to give more context. http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@123 PS2, Line 123: group1 This is wrong. The left-hand side is still a user. The client still sends the delegate user name. The server will perform an authorization to see if the delegate user belongs to the list of authorized delegated groups. http://gerrit.cloudera.org:8080/#/c/11068/2/docs/topics/impala_delegation.xml@162 PS2, Line 162: The delegation process works as follows: Please also explain the user delegation with --authorized_proxy_group_config. Also mention that --authorized_proxy_group_config does not supported ShellBasedUnixGroupsNetgroupMapping and ShellBasedUnixGroupsMapping Hadoop group mapping providers. The recommendation is to use the JNI-based mapping providers, such as JniBasedUnixGroupsMappingWithFallback and JniBasedUnixGroupsNetgroupMappingWithFallback. Reference: https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/JniFrontend.java#L665-L680 -- To view, visit http://gerrit.cloudera.org:8080/11068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6948ab2ef9b82b123f7a1f4fdc83cfb06e9f912f Gerrit-Change-Number: 11068 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zsombor Fedor Gerrit-Comment-Date: Fri, 27 Jul 2018 17:32:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/84/ : 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/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:23:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Bump Kudu version to 5211897
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Patch Set 1: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/86/ Running initial code review checks. 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/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 27 Jul 2018 17:22:14 + Gerrit-HasComments: No
[Impala-ASF-CR] Bump Kudu version to 5211897
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11071 Change subject: Bump Kudu version to 5211897 .. Bump Kudu version to 5211897 This requires changing one error message in a test due to a change in the error message on the Kudu side. Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 --- M bin/impala-config.sh M tests/query_test/test_kudu.py 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/11071/1 -- To view, visit http://gerrit.cloudera.org:8080/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Jim Apple has posted comments on this change. (
http://gerrit.cloudera.org:8080/11047 )
Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc
File be/src/exec/hdfs-avro-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc@263
PS3, Line 263: if(LIKELY(slot_byte_size == 4 || slot_byte_size == 8 ||
slot_byte_size == 16)) {
> I think I preferred the old version :). We don't need a runtime check for t
It doesn't look sort-of free to me - The Compiler Explorer reports every path
using between one and three cmp calls: https://godbolt.org/g/a1mUiD
--
To view, visit http://gerrit.cloudera.org:8080/11047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8
Gerrit-Change-Number: 11047
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 17:20:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/85/ Running initial code review checks. 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 17:16:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#3). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 138 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/3 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11047 )
Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc
File be/src/exec/hdfs-avro-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc@263
PS3, Line 263: if(LIKELY(slot_byte_size == 4 || slot_byte_size == 8 ||
slot_byte_size == 16)) {
> Added this line to exactly replicate the behavior below. PS1 & PS2 did not
I think I preferred the old version :). We don't need a runtime check for the
invariant (it sort-of happens for free below).
--
To view, visit http://gerrit.cloudera.org:8080/11047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8
Gerrit-Change-Number: 11047
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 17:12:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 ) Change subject: IMPALA-1624: Allow toggling and unsetting some command-line options inside impala-shell .. Patch Set 6: (2 comments) LGTM, except one tiny nit. I can give a +2 if the change was confirmed to to be tested on both Python 2.6 and 2.7. http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10900/6//COMMIT_MSG@15 PS6, Line 15: Added tests for all new options in test_shell_interactive.py Did you test this change on Python 2.6 as well? If yes, can you update the commit message saying tested on Python 2.6 and Python 2.7? http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10900/6/tests/shell/test_shell_interactive.py@122 PS6, Line 122: 'r' nit: use "r" to be consistent with the code in this function. -- To view, visit http://gerrit.cloudera.org:8080/10900 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763 Gerrit-Change-Number: 10900 Gerrit-PatchSet: 6 Gerrit-Owner: Le Minh Nghia Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Le Minh Nghia Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 27 Jul 2018 17:04:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Jim Apple has posted comments on this change. (
http://gerrit.cloudera.org:8080/11047 )
Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far
..
Patch Set 3:
(2 comments)
I'll wait for another +2 from you, Tim, since this changes things enough to
deserve a second quick look.
http://gerrit.cloudera.org:8080/#/c/11047/2/be/src/exec/hdfs-avro-scanner-ir.cc
File be/src/exec/hdfs-avro-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/11047/2/be/src/exec/hdfs-avro-scanner-ir.cc@264
PS2, Line 264: memset(slot, 0, slot_byte_size);
> UNLIKELY()?
done
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc
File be/src/exec/hdfs-avro-scanner-ir.cc:
http://gerrit.cloudera.org:8080/#/c/11047/3/be/src/exec/hdfs-avro-scanner-ir.cc@263
PS3, Line 263: if(LIKELY(slot_byte_size == 4 || slot_byte_size == 8 ||
slot_byte_size == 16)) {
Added this line to exactly replicate the behavior below. PS1 & PS2 did not
replicate it in the failing case.
--
To view, visit http://gerrit.cloudera.org:8080/11047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8
Gerrit-Change-Number: 11047
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 27 Jul 2018 16:50:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11047 ) Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/84/ Running initial code review checks. 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/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 27 Jul 2018 16:49:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5031: Fix undefined behavior: right-shifting too far
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11047 to look at the new patch set (#3). Change subject: IMPALA-5031: Fix undefined behavior: right-shifting too far .. IMPALA-5031: Fix undefined behavior: right-shifting too far In expr.shift, the C++ standard says of right shifts: The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand. In HdfsAvroScannerTest.DecimalTest, this is triggered, and the interesting part of the backtrace is: exec/hdfs-avro-scanner-ir.cc:272:18: runtime error: shift exponent 32 is too large for 32-bit type 'int32_t' (aka 'int') #0 0x1786f65 in HdfsAvroScanner::ReadAvroDecimal(int, unsigned char**, unsigned char*, bool, void*, MemPool*) exec/hdfs-avro-scanner-ir.cc:272:18 #1 0x1617778 in void HdfsAvroScannerTest::TestReadAvroType, bool (HdfsAvroScanner::*)(int, unsigned char**, unsigned char*, bool, void*, MemPool*), unsigned long>(bool (HdfsAvroScanner::*)(int, unsigned char**, unsigned char*, bool, void*, MemPool*), unsigned long, unsigned char*, long, DecimalValue, int, TErrorCode::type) exec/hdfs-avro-scanner-test.cc:88:20 #2 0x1605705 in void HdfsAvroScannerTest::TestReadAvroDecimal(unsigned char*, long, DecimalValue, int, TErrorCode::type) exec/hdfs-avro-scanner-test.cc:184:5 Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 --- M be/src/exec/hdfs-avro-scanner-ir.cc 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/11047/3 -- To view, visit http://gerrit.cloudera.org:8080/11047 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic5be92912198af2a5e00053ceb9a4fab43ca6bb8 Gerrit-Change-Number: 11047 Gerrit-PatchSet: 3 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/83/ : 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/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 16:34:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/82/ : 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/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 16:24:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 13: (2 comments) http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py File bin/jenkins/critique-gerrit-review.py: http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@49 PS13, Line 49: i flake8: F401 'time' imported but unused http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@121 PS13, Line 121: i flake8: E305 expected 2 blank lines after class or function definition, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 15:53:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 13: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/83/ Running initial code review checks. 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/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Fri, 27 Jul 2018 15:53:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Hello Michael Brown, David Knupp, Joe McDonnell, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11054 to look at the new patch set (#13). Change subject: IMPALA-7317: add scripts to post flake8 comments .. IMPALA-7317: add scripts to post flake8 comments The script is used by a new jenkins job gerrit-auto-critic to post comments on code reviews. Testing: This patch deliberately contains some flake8 violations so that gerrit-auto-critic will flag them. Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 --- A bin/jenkins/critique-gerrit-review.py 1 file changed, 129 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/13 -- To view, visit http://gerrit.cloudera.org:8080/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 13 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 ) Change subject: IMPALA-7317: add scripts to post flake8 comments .. Patch Set 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py File bin/jenkins/critique-gerrit-review.py: http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py@49 PS12, Line 49: i flake8: F401 'time' imported but unused http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py@122 PS12, Line 122: i flake8: E305 expected 2 blank lines after class or function definition, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Fri, 27 Jul 2018 15:50:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11054 Change subject: IMPALA-7317: add scripts to post flake8 comments .. IMPALA-7317: add scripts to post flake8 comments The script is used by a new jenkins job gerrit-auto-critic to post comments on code reviews. Testing: This patch deliberately contains some flake8 violations so that gerrit-auto-critic will flag them. Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 --- A bin/jenkins/critique-gerrit-review.py 1 file changed, 130 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/12 -- To view, visit http://gerrit.cloudera.org:8080/11054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170 Gerrit-Change-Number: 11054 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
