[Impala-ASF-CR] IMPALA-5754: Improve randomness of rand()/random()

2017-11-13 Thread Kim Jin Chul (Code Review)
Hello Jim Apple,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8355

to look at the new patch set (#5).

Change subject: IMPALA-5754: Improve randomness of rand()/random()
..

IMPALA-5754: Improve randomness of rand()/random()

Currently implementation of rand/random built-in functions
use rand_r of C library. We recognized its randomness was poor.
std::mt19937 in the C++11 standard libarary shows better randomness
than rand_r.

Testing:
rand-util-test is newly addded. It checks randomness,
deterministic and range.

Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/util/CMakeLists.txt
A be/src/util/rand-util-test.cc
A be/src/util/rand-util.cc
A be/src/util/rand-util.h
A testdata/workloads/functional-query/queries/QueryTest/random.test
M tests/query_test/test_queries.py
8 files changed, 215 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/8355/5
--
To view, visit http://gerrit.cloudera.org:8080/8355
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
Gerrit-Change-Number: 8355
Gerrit-PatchSet: 5
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread Michael Brown (Code Review)
Michael Brown has removed a vote on this change.

Change subject: Update Impala docs for 2.10 release
..


Removed Verified-1 by Impala Public Jenkins (255)
--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-6087: Revisit tests withheld from TPC-DS suite for use of TRUNCATE

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8372 )

Change subject: IMPALA-6087: Revisit tests withheld from TPC-DS suite for use 
of TRUNCATE
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q26.test
File testdata/workloads/tpcds/queries/tpcds-q26.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q26.test@5
PS5, Line 5: 78.33
> The TPC-DS spec section 7.5.3.d says "For results from AVG aggregates, the
Ack


http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q39-1.test
File testdata/workloads/tpcds/queries/tpcds-q39-1.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q39-1.test@4
PS5, Line 4: -- ADD ROUND()s TO 4th, 5th, 9th, 10th COLUMNS, USE ACTUAL RESULT 
AS EXPECTED RESULT.
> It looks like this is okay per the TPC-DS standard section 4.2.3.4.f.6: Exp
Ack



--
To view, visit http://gerrit.cloudera.org:8080/8372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79d2e34621639c8f8c4c4eb0b0944eaefca13a7a
Gerrit-Change-Number: 8372
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 23:32:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4985: use parquet stats of nested types for dynamic pruning

2017-11-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8480 )

Change subject: IMPALA-4985: use parquet stats of nested types for dynamic 
pruning
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8480/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8480/1//COMMIT_MSG@12
PS1, Line 12: A nested value is defined to
: be on a path of one or more nested types that is rooted at a
: table column.
I don't understand what that sentence means. Can you try to clarify the 
distinction between nested value and nested type?


http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@435
PS1, Line 435:   // Checks if slot refers to an array "pos" pseudo-column.
Can you add a comment explaining why checking for getColumn() == null is not 
sufficient?


http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@441
PS1, Line 441: isMapStruct
I think it would be clearer to add a isArrayStruct() method to 
CollectionStructType to emphasize that that's what we're checking.


http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@564
PS1, Line 564:
nit: the surrounding code seems to omit this space.


http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@567
PS1, Line 567:   for (Expr pred: entry.getValue()) {
 : if (pred instanceof BinaryPredicate) {
 :   tryComputeBinaryMinMaxPredicate(analyzer, 
(BinaryPredicate) pred);
 : } else if (pred instanceof InPredicate) {
 :   tryComputeInListMinMaxPredicate(analyzer, 
(InPredicate) pred);
 : }
 :   }
This looks like a duplication of the above loop. Adding additional predicates 
in the future may require changing both loops. Have you considered factoring it 
into it's own method?


http://gerrit.cloudera.org:8080/#/c/8480/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1109
PS1, Line 1109: slot.getColumn() == null
Is this another check for a pos slot?


http://gerrit.cloudera.org:8080/#/c/8480/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/8480/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test@141
PS1, Line 141: Basics test
I'm not sure I understand what Basics means. Can you elaborate? I think we 
often order tests by ascending complexity so that the simpler ones fail before 
the complex ones.


http://gerrit.cloudera.org:8080/#/c/8480/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/8480/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@460
PS1, Line 460: 
Does this remove the trailing newline?



--
To view, visit http://gerrit.cloudera.org:8080/8480
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe
Gerrit-Change-Number: 8480
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 14 Nov 2017 00:10:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction

2017-11-13 Thread Csaba Ringhofer (Code Review)
Hello Lars Volker, Tim Armstrong,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8436

to look at the new patch set (#5).

Change subject: IMPALA-6054: Parquet dictionary pages should be freed on 
dictionary construction
..

IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction

During dictionary constructon, most types are copied from the parquet
dictionary page, but StringValues keep pointers to it. In this case,
the dictionary page must be kept and attached to the last row batch
that references it. In case of other types, it is safe do delete
the dictionary page after the construction of the dictionary.

This patch contains two optimizations:
- dictionary pages are deleted as soon as possible for non string types
- in non-compressed and non-string case, an unnecessary copy is avoided

Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004
---
M be/src/exec/parquet-column-readers.cc
1 file changed, 33 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8436/5
--
To view, visit http://gerrit.cloudera.org:8080/8436
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004
Gerrit-Change-Number: 8436
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-3613: Avoid topic updates to unregistered subscriber instances

2017-11-13 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8449 )

Change subject: IMPALA-3613: Avoid topic updates to unregistered subscriber 
instances
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8449/6/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/8449/6/be/src/statestore/statestore.h@383
PS6, Line 383: std::pair
> once we have two level pair, I think it's time to start naming the fields.
Done



--
To view, visit http://gerrit.cloudera.org:8080/8449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0329ae7d23dc6e9b04b7bc3ee8d89cbc73756f65
Gerrit-Change-Number: 8449
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 14 Nov 2017 00:18:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread John Russell (Code Review)
John Russell has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..

Update Impala docs for 2.10 release

Commit hash = 40ec6d0080638efaf3260672ab54ea4674896c5e

Several new HTML files for this release.

Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Reviewed-on: http://gerrit.cloudera.org:8080/8511
Reviewed-by: Michael Brown 
Tested-by: John Russell 
---
M docs/build/html/index.html
M docs/build/html/topics/impala_adls.html
M docs/build/html/topics/impala_alter_table.html
M docs/build/html/topics/impala_array.html
A docs/build/html/topics/impala_buffer_pool_limit.html
M docs/build/html/topics/impala_compute_stats.html
M docs/build/html/topics/impala_config_options.html
M docs/build/html/topics/impala_connecting.html
M docs/build/html/topics/impala_create_table.html
M docs/build/html/topics/impala_datetime_functions.html
A docs/build/html/topics/impala_default_spillable_buffer_size.html
M docs/build/html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/html/topics/impala_fixed_issues.html
M docs/build/html/topics/impala_hbase.html
M docs/build/html/topics/impala_hints.html
M docs/build/html/topics/impala_incompatible_changes.html
M docs/build/html/topics/impala_joins.html
M docs/build/html/topics/impala_known_issues.html
M docs/build/html/topics/impala_kudu.html
M docs/build/html/topics/impala_logging.html
M docs/build/html/topics/impala_map.html
M docs/build/html/topics/impala_math_functions.html
A docs/build/html/topics/impala_max_row_size.html
A docs/build/html/topics/impala_min_spillable_buffer_size.html
M docs/build/html/topics/impala_new_features.html
M docs/build/html/topics/impala_partitioning.html
M docs/build/html/topics/impala_perf_joins.html
M docs/build/html/topics/impala_perf_stats.html
M docs/build/html/topics/impala_query_options.html
M docs/build/html/topics/impala_reserved_words.html
M docs/build/html/topics/impala_scalability.html
M docs/build/html/topics/impala_select.html
M docs/build/html/topics/impala_shell_commands.html
M docs/build/html/topics/impala_shell_running_commands.html
M docs/build/html/topics/impala_ssl.html
M docs/build/html/topics/impala_string_functions.html
M docs/build/html/topics/impala_struct.html
M docs/build/html/topics/impala_subqueries.html
A docs/build/html/topics/impala_tablesample.html
M docs/build/html/topics/impala_upgrading.html
M docs/build/html/topics/impala_views.html
A docs/build/impala-2.10.pdf
M impala-docs.html
43 files changed, 3,570 insertions(+), 1,118 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  John Russell: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: merged
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-6087: Revisit tests withheld from TPC-DS suite for use of TRUNCATE

2017-11-13 Thread Mostafa Mokhtar (Code Review)
Mostafa Mokhtar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8372 )

Change subject: IMPALA-6087: Revisit tests withheld from TPC-DS suite for use 
of TRUNCATE
..


Patch Set 5:

(2 comments)

Do you have 3 runs showing that there is no new flaky test cases?

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q77a.test
File testdata/workloads/tpcds/queries/tpcds-q77a.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q77a.test@4
PS5, Line 4: -- FIXED. USE ACTUAL RESULT AS EXPECTED RESULT
Please add explanation for what was different.


http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q86a.test
File testdata/workloads/tpcds/queries/tpcds-q86a.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q86a.test@4
PS5, Line 4: -- FIXED. USE ACTUAL RESULT AS EXPECTED RESULT
What was the difference?



--
To view, visit http://gerrit.cloudera.org:8080/8372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79d2e34621639c8f8c4c4eb0b0944eaefca13a7a
Gerrit-Change-Number: 8372
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Tue, 14 Nov 2017 00:43:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Expose $IMPALA MAVEN OPTIONS for configuring Maven.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8496 )

Change subject: Expose $IMPALA_MAVEN_OPTIONS for configuring Maven.
..

Expose $IMPALA_MAVEN_OPTIONS for configuring Maven.

With this commit, $IMPALA_MAVEN_OPTIONS is used by bin/mvn-quiet.sh
to configure Maven slightly. The default is no extra options.

This is handy for giving Maven a settings file with the "-s" flag, to
control, for example, repositories and their mirrors. In fact, I
considered exposing IMPALA_MAVEN_SETTINGS_FILE explicitly, but decided
that the generic option would be as good.

It's useful to customize how Maven works, especially
to provide a settings file with repository mirrors.

Change-Id: I2c62185476fd2388c7cda8884276b79a77370127
Reviewed-on: http://gerrit.cloudera.org:8080/8496
Reviewed-by: Joe McDonnell 
Tested-by: Impala Public Jenkins
---
M bin/impala-config.sh
M bin/mvn-quiet.sh
M testdata/bin/generate-block-ids.sh
M testdata/bin/generate-load-nested.sh
M testdata/bin/split-hbase.sh
5 files changed, 14 insertions(+), 8 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2c62185476fd2388c7cda8884276b79a77370127
Gerrit-Change-Number: 8496
Gerrit-PatchSet: 5
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6087: Revisit tests withheld from TPC-DS suite for use of TRUNCATE

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8372 )

Change subject: IMPALA-6087: Revisit tests withheld from TPC-DS suite for use 
of TRUNCATE
..


Patch Set 6:

(2 comments)

> Patch Set 5:
>
> (2 comments)
>
> Do you have 3 runs showing that there is no new flaky test cases?

Yes; two local, and one in progress at 
jenkins.impala.io/job/gerrit-verify-dryrun-external/26/.

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q77a.test
File testdata/workloads/tpcds/queries/tpcds-q77a.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q77a.test@4
PS5, Line 4: -- FIXED, MADE UNIFORM 1/100 PRECISION. USE ACTUAL RESULT AS 
EXPECTED RESULT
> Please add explanation for what was different.
Done


http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q86a.test
File testdata/workloads/tpcds/queries/tpcds-q86a.test:

http://gerrit.cloudera.org:8080/#/c/8372/5/testdata/workloads/tpcds/queries/tpcds-q86a.test@4
PS5, Line 4: -- FIXED, MADE UNIFORM 1/100 PRECISION. USE ACTUAL RESULT AS 
EXPECTED RESULT
> What was the difference?
Done



--
To view, visit http://gerrit.cloudera.org:8080/8372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79d2e34621639c8f8c4c4eb0b0944eaefca13a7a
Gerrit-Change-Number: 8372
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Tue, 14 Nov 2017 02:02:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5976: Remove equivalence class computation in FE

2017-11-13 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8317 )

Change subject: IMPALA-5976: Remove equivalence class computation in FE
..


Patch Set 4:

(24 comments)

http://gerrit.cloudera.org:8080/#/c/8317/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8317/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@91
PS3, Line 91: /**
> I think If the predicate transfer is the starting point to consider whether
Fair point. I was thinking of a definition like this:

Slot A has a value transfer to slot B if for all rows containing both slots 
slot B has the same value as slot A or the tuple containing slot B is NULL.


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@102
PS4, Line 102:  * Slot value transfer:
Slot value transfers:


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@105
PS4, Line 105:  * Its symmetric closure is a equivalence relation. Its 
equivalence class is called slot
What does "Its" refer to here?

I think it's easier to state less formally:

Each slot is contained in exactly one equivalence class. A slot equivalence 
class is a set of slots where each pair of slots has a mutual value transfer. 
Equivalence classes are assigned an arbitrary id to distinguish them from 
another.


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1527
PS4, Line 1527:   // select * from (select A.a, B.b from A left join B on 
A.a=B.b) v where b is null
to drive it home even further use "B.col is null" as the predicate to show that 
the NULL-checking predicate is unrelated to the slots participating in the 
equivalence


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1648
PS4, Line 1648: // A map from equivalence class IDs to equivalence 
classese. The equivalence classes
typo: classes


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1813
PS4, Line 1813:* Returns a map of slot equivalence classes on the set of 
slots in given tuples.
the given tuples


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1838
PS4, Line 1838:* propagation. Each mapping assigns every slot in srcSids to 
slot in destTid which has
Each mapping assigns every slot in srcSids to a slot in destTid which has a 
value transfer from srcSid.


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1989
PS4, Line 1989: + "\n" + tc + "Condensed Graph:\n" + condensedTc);
move the first "\n" to previous line


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1998
PS4, Line 1998: // transform equality predicates into a transfer graph
remove (pretty clear from function comment)


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@691
PS4, Line 691:   interface SlotRefComparator {
Move this into SlotRef since it's SlotRef specific?


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@696
PS4, Line 696:* Returns if this expr matches 'that'. Two exprs match if:
Returns true if this expr matches 'that'


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@699
PS4, Line 699:* 2. For every pair of corresponding SlotRef, 
slotRefCmp.matches returns true.
For every pair of corresponding SlotRefs, slotRefCmp.matches() returns true.


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@700
PS4, Line 700:* 3. For every pair of corresponding non-SlotRef exprs, 
localEquals returns true.
localEquals()

(we generally use () for function names to make it clear we are referring to a 
function)


http://gerrit.cloudera.org:8080/#/c/8317/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@719
PS4, Line 719: if (fn_ == null && that.fn_ == null) return true;
I think we should separate matches() and localEquals() more cleanly. I think 
localEquals() should be non-abstract and have a default implementation that 
checks the type and fn_ of this and that. Subclasses can override and call 
super.localEquals() first.



[Impala-ASF-CR] IMPALA-3613: Avoid topic updates to unregistered subscriber instances

2017-11-13 Thread Bharath Vissapragada (Code Review)
Hello Sailesh Mukil, Dimitris Tsirogiannis, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8449

to look at the new patch set (#7).

Change subject: IMPALA-3613: Avoid topic updates to unregistered subscriber 
instances
..

IMPALA-3613: Avoid topic updates to unregistered subscriber instances

Bug:

Without this patch, when a subscriber repeatedly reconnects to the
statestore, the latter queues the initial heartbeat message and a
bunch of topic updates to every instance of the registered subscriber.
These queued updates are eventually picked up by the heartbeating/topic
update threads and the corresponding RPCs are made to the subscribers.
The subscriber then rejects these updates since they were meant for an
earlier registration. This is usually possible if the subscriber has
some network problems leading to failing RPCs.

Such a node is eventually marked by the statestore as bad, but depending
on the configurations set, the issue can snowball into DDOS kind of
attack when the entire thread pool of heartbeating/topic updates is
filled with instances from the problematic host. This can result in
the statestore missing timely heartbeats to other subscribers making
them reconnect. This worsens the situation and the resulting topic
updates for the reconnects will fully saturate the network on the
statestore host, until the statestore daemon is restarted.

Fix:

This patch maps topic updates/heartbeats to a specific subscriber
registered instance rather to a subscriber id (that stays same across
reconnects). That way, when we encounter a topic update that was meant to
a stale subscriber, we can simply reject it.

Testing:

Tested this locally by adding relevant logging. I made the subscribers
to reconnect aggressively(a) and delaying heartbeats from the statestore
side (b,c).

(a) --statestore_subscriber_timeout_seconds=1
(b) --statestore_max_missed_heartbeats=1000
(c) --statestore_heartbeat_frequency_ms=6

Change-Id: I0329ae7d23dc6e9b04b7bc3ee8d89cbc73756f65
---
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
4 files changed, 91 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/8449/7
--
To view, visit http://gerrit.cloudera.org:8080/8449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0329ae7d23dc6e9b04b7bc3ee8d89cbc73756f65
Gerrit-Change-Number: 8449
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] Expose $IMPALA MAVEN OPTIONS for configuring Maven.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8496 )

Change subject: Expose $IMPALA_MAVEN_OPTIONS for configuring Maven.
..


Patch Set 4: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/8496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c62185476fd2388c7cda8884276b79a77370127
Gerrit-Change-Number: 8496
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Nov 2017 01:29:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6087: Revisit tests withheld from TPC-DS suite for use of TRUNCATE

2017-11-13 Thread Tim Wood (Code Review)
Hello Greg Rahn, Matthew Mulder, Michael Brown, Mostafa Mokhtar,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8372

to look at the new patch set (#6).

Change subject: IMPALA-6087: Revisit tests withheld from TPC-DS suite for use 
of TRUNCATE
..

IMPALA-6087: Revisit tests withheld from TPC-DS suite for use of TRUNCATE

The .test files in this commit are ones held out from IMPALA-5376
because of observed anomalies with returned decimal precision that I
previously controlled with TRUNCATE().  This ticket was filed after team
members expressed a preference not to use TRUNCATE() and to use
actual results generated within numerical range of expected,
unless the results could not be controlled otherwise.  Rounds of testing
for this commit show that the earlier anomalies no longer occur,
clearing the way for their inclusion in our TPC-DS suite.  It has been
observed that these tests tend to fail with the DECIMAL_V2 option set
(unless the test does so explicitly).

Testing: The gerrit_verify_dryrun_external (build #24) job passes with this
change.  The fix (rebased hereon) to IMPALA-6106 (finally) cleared up a
source of flapping for these tests.

Change-Id: I79d2e34621639c8f8c4c4eb0b0944eaefca13a7a
---
A testdata/workloads/tpcds/queries/tpcds-q26.test
A testdata/workloads/tpcds/queries/tpcds-q28.test
M testdata/workloads/tpcds/queries/tpcds-q39-1.test
M testdata/workloads/tpcds/queries/tpcds-q39-2.test
A testdata/workloads/tpcds/queries/tpcds-q47.test
A testdata/workloads/tpcds/queries/tpcds-q57.test
A testdata/workloads/tpcds/queries/tpcds-q59.test
M testdata/workloads/tpcds/queries/tpcds-q61.test
A testdata/workloads/tpcds/queries/tpcds-q63.test
M testdata/workloads/tpcds/queries/tpcds-q77a.test
M testdata/workloads/tpcds/queries/tpcds-q78.test
M testdata/workloads/tpcds/queries/tpcds-q86a.test
M tests/query_test/test_tpcds_queries.py
13 files changed, 851 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/8372/6
--
To view, visit http://gerrit.cloudera.org:8080/8372
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I79d2e34621639c8f8c4c4eb0b0944eaefca13a7a
Gerrit-Change-Number: 8372
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Wood 


[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu

2017-11-13 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7793 )

Change subject: IMPALA-4252: Min-max runtime filters for Kudu
..


Patch Set 14: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/7793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
Gerrit-Change-Number: 7793
Gerrit-PatchSet: 14
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Anonymous Coward #345
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 13 Nov 2017 23:28:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3613: Avoid topic updates to unregistered subscriber instances

2017-11-13 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8449 )

Change subject: IMPALA-3613: Avoid topic updates to unregistered subscriber 
instances
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8449/7/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/8449/7/be/src/statestore/statestore.h@394
PS7, Line 394:   typedef std::pair 
ScheduledSubscriberUpdate;
I meant flatten both pairs -- i.e. turn ScheduledSubscriberUpdate into a struct 
(with three fields).



--
To view, visit http://gerrit.cloudera.org:8080/8449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0329ae7d23dc6e9b04b7bc3ee8d89cbc73756f65
Gerrit-Change-Number: 8449
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 14 Nov 2017 05:40:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5754: Improve randomness of rand()/random()

2017-11-13 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8355 )

Change subject: IMPALA-5754: Improve randomness of rand()/random()
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8355/3/be/src/util/rand-util.h
File be/src/util/rand-util.h:

http://gerrit.cloudera.org:8080/#/c/8355/3/be/src/util/rand-util.h@28
PS3, Line 28:   static double generate(std::mt19937& generator, const double 
min, const double max);
> It will not necessarily be possible or wise to replace all IMPALA-4954 uses
Thanks for adding the e2e tests. I'd still recommend the file removals 
referenced in this comment.



--
To view, visit http://gerrit.cloudera.org:8080/8355
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
Gerrit-Change-Number: 8355
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 14 Nov 2017 05:25:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction

2017-11-13 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8436 )

Change subject: IMPALA-6054: Parquet dictionary pages should be freed on 
dictionary construction
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8436/4/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/8436/4/be/src/exec/parquet-column-readers.cc@934
PS4, Line 934: null
> nullptr
Done


http://gerrit.cloudera.org:8080/#/c/8436/4/be/src/exec/parquet-column-readers.cc@942
PS4, Line 942:   // 3. If the column type is not string, and the dictionary 
page is not compressed,
> I'm not 100% confident that we have test coverage for this case - we have s
I have checked, and this branch is executed if exploration strategy > core.


http://gerrit.cloudera.org:8080/#/c/8436/4/be/src/exec/parquet-column-readers.cc@944
PS4, Line 944:   ScopedBuffer 
uncompressed_buffer(parent_->dictionary_pool_->mem_tracker());
> nit: unnecessary blank line
Done


http://gerrit.cloudera.org:8080/#/c/8436/4/be/src/exec/parquet-column-readers.cc@964
PS4, Line 964:
> nit: != nullptr (we prefer explicit checks against null to make it visually
Done



--
To view, visit http://gerrit.cloudera.org:8080/8436
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004
Gerrit-Change-Number: 8436
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Nov 2017 00:14:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1459/


--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 17:52:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

2017-11-13 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 17:51:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu

2017-11-13 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7793 )

Change subject: IMPALA-4252: Min-max runtime filters for Kudu
..


Patch Set 14:

(2 comments)

looked through the headers and thrift and it looks fine to me. Just a question 
about the query options.

http://gerrit.cloudera.org:8080/#/c/7793/14/be/src/exec/kudu-util.cc
File be/src/exec/kudu-util.cc:

http://gerrit.cloudera.org:8080/#/c/7793/14/be/src/exec/kudu-util.cc@116
PS14, Line 116: Status ConvertTimestampValue(const TimestampValue* tv, int64_t* 
ts_micros) {
static


http://gerrit.cloudera.org:8080/#/c/7793/14/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

http://gerrit.cloudera.org:8080/#/c/7793/14/common/thrift/ImpalaInternalService.thrift@203
PS14, Line 203:   41: optional i32 max_num_runtime_filters = 10
so this (and two options below) only apply to bloom but not min/max filters? is 
that going to be clear to a user? hopefully we don't expect most to use these 
options, so maybe we should rename them?



--
To view, visit http://gerrit.cloudera.org:8080/7793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
Gerrit-Change-Number: 7793
Gerrit-PatchSet: 14
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Anonymous Coward #345
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 13 Nov 2017 18:59:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Remove unnecessary 'incubator' from URLs

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8524 )

Change subject: [DOCS] Remove unnecessary 'incubator' from URLs
..


Patch Set 2: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/8524
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0f8d51b147094e629e60c4a8c5aecbb6cdb6a8e
Gerrit-Change-Number: 8524
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 18:59:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Remove unnecessary 'incubator' from URLs

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8524 )

Change subject: [DOCS] Remove unnecessary 'incubator' from URLs
..

[DOCS] Remove unnecessary 'incubator' from URLs

URLs like https://impala.incubator.apache.org/ are aliases with
https://impala.apache.org/, so we can use the latter and avoid making
any changes if or when Impala graduates from the incubator.

Change-Id: If0f8d51b147094e629e60c4a8c5aecbb6cdb6a8e
Reviewed-on: http://gerrit.cloudera.org:8080/8524
Reviewed-by: Michael Brown 
Tested-by: Impala Public Jenkins
---
M docs/impala_keydefs.ditamap
M docs/topics/impala_langref_unsupported.xml
2 files changed, 5 insertions(+), 5 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8524
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If0f8d51b147094e629e60c4a8c5aecbb6cdb6a8e
Gerrit-Change-Number: 8524
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-2235: Fix current db when shell auto-reconnects

2017-11-13 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8368 )

Change subject: IMPALA-2235: Fix current db when shell auto-reconnects
..


Patch Set 3: Code-Review+1

(3 comments)

Thanks for the updates. This looks fine to me, with 2 minor issues:

* Please re-work the first line of the commit message; it doesn't make sense to 
me. (Or maybe I'm missing something, in which case just let me know.)

* If you're using NamedTemporaryFile to just get a filename, I suggested a 
shorter solution. There are two cases in this diff where you could take 
advantage of that.

I'm fine with both of those changes going in without further review; they're 
very minor.

http://gerrit.cloudera.org:8080/#/c/8368/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8368/3//COMMIT_MSG@9
PS3, Line 9: When precmd tested the connection it didn't validate that if we are
This sentence didn't parse for me. Specifically, I'm not sure what "that" in 
"it didn't validate that" is referring to.


http://gerrit.cloudera.org:8080/#/c/8368/3/tests/custom_cluster/test_shell_interactive_reconnect.py
File tests/custom_cluster/test_shell_interactive_reconnect.py:

http://gerrit.cloudera.org:8080/#/c/8368/3/tests/custom_cluster/test_shell_interactive_reconnect.py@36
PS3, Line 36: tempfile = NamedTemporaryFile(delete=False)
: tempfile.close()
: cls.tempfile_name = tempfile.name
Minor: I think this is just:

cls.tempfile_name = tempfile.mktemp()

Or did you use NamedTemporaryFile to create the file too, and that's important 
somehow?


http://gerrit.cloudera.org:8080/#/c/8368/3/tests/shell/util.py
File tests/shell/util.py:

http://gerrit.cloudera.org:8080/#/c/8368/3/tests/shell/util.py@121
PS3, Line 121:   """ Moves back history file from given filepath """
mention that this is a no-op if filepath doesn't exist.



--
To view, visit http://gerrit.cloudera.org:8080/8368
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I40dfa00ba0314d356fe8617446f516505c925e5e
Gerrit-Change-Number: 8368
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 13 Nov 2017 19:14:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6160: Rework query execution to handle multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Hello Matthew Mulder, Michael Brown, Jim Apple,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8513

to look at the new patch set (#2).

Change subject: IMPALA-6160: Rework query execution to handle multiple 
statements in a Query object.
..

IMPALA-6160: Rework query execution to handle multiple statements in a Query 
object.

Testing:
- Reproduced problem with bin/run-workload.py.
- Ran bin/run-workload.py --workloads=tpch,targeted-perf,tpcds
  --impalads=localhost:21000,localhost:21001,localhost:21002
  --results_json_file=$PWD/perf_results/IMPALA-6160.json
  --query_iterations=3 --table_formats=parquet/none --plan_first 
--query_names='.*'
  (Close to command line that single_node_perf_run.py builds.)
- Manually reviewed perf_results/IMPALA-6160.json to verify presence of plans
  and proper splitting of query batches.

Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
---
M tests/performance/query_executor.py
1 file changed, 55 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8513/2
--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 


[Impala-ASF-CR] [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web UI.

2017-11-13 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8529


Change subject: [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web 
UI.
..

[PREVIEW] IMPALA-4886: Expose table metrics in the catalog web UI.

The following changes are included in this commit:
* Adds a lightweight framework for registering metrics in the JVM.
* Adds table-level metrics and enables these metrics to be exposed
through the catalog web UI.
* Adds a CatalogUsageMonitor class that monitors and reports the catalog
usage in terms of the tables with the highest memory requirements and
the tables with the highest number of metadata operations. The catalog
usage information is exposed in the /catalog page of the catalog web UI.

Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/catalog/catalog.cc
M be/src/catalog/catalog.h
M common/thrift/Frontend.thrift
M common/thrift/JniCatalog.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
A fe/src/main/java/org/apache/impala/catalog/CatalogUsageMonitor.java
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
A fe/src/main/java/org/apache/impala/common/Metrics.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
A fe/src/main/java/org/apache/impala/util/TopNCache.java
M www/catalog.tmpl
A www/table_metrics.tmpl
21 files changed, 953 insertions(+), 73 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8529/1
--
To view, visit http://gerrit.cloudera.org:8080/8529
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
Gerrit-Change-Number: 8529
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 


[Impala-ASF-CR] [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web UI.

2017-11-13 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8529 )

Change subject: [PREVIEW] IMPALA-4886: Expose table metrics in the catalog web 
UI.
..


Patch Set 1:

Screenshots of the catalog web UI to illustrate the changes:
https://cloudera.box.com/s/tgvt0yc3mfhvc25i2tm30m3h49769jfp
https://cloudera.box.com/s/hc0ru8q2ajfne6gzy5x4s5ri5p6kgawz


--
To view, visit http://gerrit.cloudera.org:8080/8529
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
Gerrit-Change-Number: 8529
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Mon, 13 Nov 2017 19:40:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6160: Rework query execution to handle multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Rework query execution to handle multiple 
statements in a Query object.
..


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@45
PS1, Line 45: query options' names
> I'm a bit confused about this - I don't see query options in these regexes.
Residue; will correct.


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52: re.I
> I think spelling out IGNORECASE makes it much more readable.
Ack


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52: SELECT
> SELECT starts statements that are not DDL or DML, yes?
I consider SELECT as DML (data manipulation, but not mutation. :)  I could call 
this DDL_CRUD_PATTERN if it's that important.


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@186
PS1, Line 186:   """Executes a query.
> Can you change the comments and member variable to pluralize "query"?
Ack


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@224
PS1, Line 224: a set of SQL statements
> This looks very general to me. Might you be able to get by with something s
I don't want to limit the use cases to just SET, when there's nothing really 
unique about it.  This routine would get more complicated(*), add needless 
error conditions, and hasten the day when it would need changing again.

* What if it's 2 SETs and a query?  A query, a SET and a query?  There's no 
need to reject these legal batches.  And, it turns out, the performance harness 
assumes results are correct without checking.


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@229
PS1, Line 229: timing
> How DOES the timing work for this object? There doesn't seem to be any timi
The execution logic in query_exec_functions.py sets the start_time and 
time_taken fields of the various QueryResult objects.  The loop at l. 256 below 
does overwrite the reference to the query result for statements 1..n-1 and 
leaves the result of the last as the result of the batch.  I will document 
this.  So far, there's no requirement for the higher layers to receive a list 
of results for the batch.


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@231
PS1, Line 231:
 : This function originally assumed that self.query contained 
only a single
 : query statement, but that assumption is not valid for a test 
file that
 : contains, e.g., a SET DECIMAL_V2=n; statement before the 
actual query for the
 : test.
> nit: I think you can omit this.
Ack



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 19:40:00 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Remove unnecessary 'incubator' from URLs

2017-11-13 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8524 )

Change subject: [DOCS] Remove unnecessary 'incubator' from URLs
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8524
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0f8d51b147094e629e60c4a8c5aecbb6cdb6a8e
Gerrit-Change-Number: 8524
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 18:39:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Remove unnecessary 'incubator' from URLs

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8524 )

Change subject: [DOCS] Remove unnecessary 'incubator' from URLs
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/172/


--
To view, visit http://gerrit.cloudera.org:8080/8524
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0f8d51b147094e629e60c4a8c5aecbb6cdb6a8e
Gerrit-Change-Number: 8524
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 18:54:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Zach Amsden (Code Review)
Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1:

Sorry, didn't realize this was the upstream commit.  What I meant to say was we 
had exactly the same issue where the wrong impala-lzo bits were getting used.  
The result was exactly the same compile failure:

hdfs-lzo-text-scanner.cc:589:44: error: no matching function for call to 
'impala::ScannerContext::ReleaseCompletedResources(bool)'
   context_->ReleaseCompletedResources(false);


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:44:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding

2017-11-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8267 )

Change subject: IMPALA-4177,IMPALA-6039: batched bit reading and rle decoding
..


Patch Set 14: Code-Review+2

Yes, I had another look at the delta since PS12 and it looks good to me.


--
To view, visit http://gerrit.cloudera.org:8080/8267
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I35de0cf80c86f501c4a39270afc8fb8111552ac6
Gerrit-Change-Number: 8267
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:52:55 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/173/


--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:52:25 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-docs-submit/173/


--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:59:19 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread John Russell (Code Review)
John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..


Patch Set 3: Verified+1

Oops, ran the verification job by mistake instead of +1'ing it through gerrit.


--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 23:04:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1:

Ping?

Thanks!


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:03:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52:
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py@190
PS3, Line 190: query (Query): SQL query/queries (batch, ;-delimited) to be 
executed
> One or more; does the word "batch" not imply that?
Done



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:24:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6160: Allow multiple statements in a Query object.
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG@14
PS1, Line 14:   --query_iterations=3 --table_formats=parquet/none --plan_first
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@45
PS1, Line 45:
> Residue; will correct.
Done


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52:
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@186
PS1, Line 186:   """Executes one or more queries.
> Ack
Done


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@229
PS1, Line 229: le" qu
> The execution logic in query_exec_functions.py sets the start_time and time
Done


http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@231
PS1, Line 231: metadata loading required for planning.
 :
 : This function furnishes a query result object in 
self._result, for the last
 : query in the batch ONLY.
 : """
> Ack
Done



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 20:48:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..


Patch Set 3:

(2 comments)

Just nits left

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52:
> I consider SELECT as DML (data manipulation, but not mutation. :)  I could
I get that you consider SELECT as DML, but I don't think most other people do, 
so I'd suggest renaming as a favor to them.


http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py@190
PS3, Line 190: query (Query): SQL query (batch, ;-delimited) to be executed
This is still queries, not a single query, right?



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 20:53:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:07:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Hello Matthew Mulder, Michael Brown, Jim Apple,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8513

to look at the new patch set (#4).

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..

IMPALA-6160: Allow multiple statements in a Query object.

Testing:
- Reproduced problem with bin/run-workload.py.
- Ran bin/run-workload.py --workloads=tpch,targeted-perf,tpcds
  --impalads=localhost:21000,localhost:21001,localhost:21002
  --results_json_file=$PWD/perf_results/IMPALA-6160.json
  --query_iterations=3 --table_formats=parquet/none --plan_first
  --query_names='.*' (Close to command line that single_node_perf_run.py
  builds.)
- Manually reviewed perf_results/IMPALA-6160.json to verify presence of
  plans and proper splitting of query batches.

Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
---
M tests/performance/query_executor.py
1 file changed, 56 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8513/4
--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Hello Matthew Mulder, Michael Brown, Jim Apple,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8513

to look at the new patch set (#3).

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..

IMPALA-6160: Allow multiple statements in a Query object.

Testing:
- Reproduced problem with bin/run-workload.py.
- Ran bin/run-workload.py --workloads=tpch,targeted-perf,tpcds
  --impalads=localhost:21000,localhost:21001,localhost:21002
  --results_json_file=$PWD/perf_results/IMPALA-6160.json
  --query_iterations=3 --table_formats=parquet/none --plan_first
  --query_names='.*' (Close to command line that single_node_perf_run.py
  builds.)
- Manually reviewed perf_results/IMPALA-6160.json to verify presence of
  plans and proper splitting of query batches.

Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
---
M tests/performance/query_executor.py
1 file changed, 56 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8513/3
--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 


[Impala-ASF-CR] IMPALA-6160: Rework query execution to handle multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Rework query execution to handle multiple 
statements in a Query object.
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6160: Rework query execution to handle multiple statements 
in a Query object.
> nit: For the subject line, please keep it under 72 characters and one line.
Ack


http://gerrit.cloudera.org:8080/#/c/8513/1//COMMIT_MSG@14
PS1, Line 14:   --query_iterations=3 --table_formats=parquet/none --plan_first 
--query_names='.*'
> nit: can you wrap at 72 characters, here? If you use emacs to write your co
Ack



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 20:44:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1460/


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:08:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..

IMPALA-4704: Turns on client connections when local catalog initialized.

Currently, impalad starts beeswax and hs2 servers even if the
catalog has not yet been initialized. As a result, client
connections see an error message stating that the impalad
is not yet ready.

This patch changes the impalad startup sequence to wait
until the catalog is received before opening beeswax and hs2 ports
and starting their servers.

Testing:
- python e2e tests that start a cluster without a catalog
  and check that client connections are rejected as expected.

Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Reviewed-on: http://gerrit.cloudera.org:8080/8202
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/global-flags.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M bin/start-impala-cluster.py
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_catalog_wait.py
M tests/custom_cluster/test_coordinators.py
20 files changed, 346 insertions(+), 214 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 25
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-4704: Turns on client connections when local catalog initialized.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8202 )

Change subject: IMPALA-4704: Turns on client connections when local catalog 
initialized.
..


Patch Set 24: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/8202
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 24
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:14:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6160: Allow multiple statements in a Query object.

2017-11-13 Thread Tim Wood (Code Review)
Tim Wood has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8513 )

Change subject: IMPALA-6160: Allow multiple statements in a Query object.
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/1/tests/performance/query_executor.py@52
PS1, Line 52:
> I get that you consider SELECT as DML, but I don't think most other people
Ack


http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py
File tests/performance/query_executor.py:

http://gerrit.cloudera.org:8080/#/c/8513/3/tests/performance/query_executor.py@190
PS3, Line 190: query (Query): SQL query (batch, ;-delimited) to be executed
> This is still queries, not a single query, right?
One or more; does the word "batch" not imply that?

I'll put "query/queries".



--
To view, visit http://gerrit.cloudera.org:8080/8513
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac86af181b7c42655f21d2c1efd4652dd35d9297
Gerrit-Change-Number: 8513
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:12:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3613: Avoid topic updates to unregistered subscriber instances

2017-11-13 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8449 )

Change subject: IMPALA-3613: Avoid topic updates to unregistered subscriber 
instances
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8449/6/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/8449/6/be/src/statestore/statestore.h@383
PS6, Line 383: std::pair
once we have two level pair, I think it's time to start naming the fields. How 
about defining a struct for this thing instead?



--
To view, visit http://gerrit.cloudera.org:8080/8449
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0329ae7d23dc6e9b04b7bc3ee8d89cbc73756f65
Gerrit-Change-Number: 8449
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:13:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6121: remove I/O mgr request context cache

2017-11-13 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8408 )

Change subject: IMPALA-6121: remove I/O mgr request context cache
..


Patch Set 8: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8408/8/be/src/runtime/disk-io-mgr-reader-context.h
File be/src/runtime/disk-io-mgr-reader-context.h:

http://gerrit.cloudera.org:8080/#/c/8408/8/be/src/runtime/disk-io-mgr-reader-context.h@27
PS8, Line 27:  but some clients may need to include this header, e.g. to make 
the
: /// unique_ptr destructor work correctly.
: ///
that's really unfortunate, but the change is a net win.


http://gerrit.cloudera.org:8080/#/c/8408/7/be/src/runtime/disk-io-mgr.h
File be/src/runtime/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8408/7/be/src/runtime/disk-io-mgr.h@654
PS7, Line 654:   std::unique_ptr 
RegisterContext(MemTracker* reader_mem_tracker);
> I think it's an implementation detail the RegisterContext() doesn't registe
I wasn't thinking in terms of implementation. I was thinking in terms of the 
interface -- how can a context be "registered" when the caller doesn't pass a 
context to be registered?

Anyway, I'm fine with leaving the terminology alone in this change.


http://gerrit.cloudera.org:8080/#/c/8408/8/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

http://gerrit.cloudera.org:8080/#/c/8408/8/be/src/runtime/runtime-state.h@39
PS8, Line 39: class DiskIoRequestContext;
delete



--
To view, visit http://gerrit.cloudera.org:8080/8408
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108
Gerrit-Change-Number: 8408
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:05:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu

2017-11-13 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7793 )

Change subject: IMPALA-4252: Min-max runtime filters for Kudu
..


Patch Set 14: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7793/14/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

http://gerrit.cloudera.org:8080/#/c/7793/14/common/thrift/ImpalaInternalService.thrift@203
PS14, Line 203:   41: optional i32 max_num_runtime_filters = 10
> so this (and two options below) only apply to bloom but not min/max filters
I see you addressed this in a doc you had already linked:
https://docs.google.com/document/d/1G-SPZelateebNxzVb67urEVtjc5Itw-B_jjfS85bSCE/edit?usp=sharing

That reasoning seems okay with me.



--
To view, visit http://gerrit.cloudera.org:8080/7793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
Gerrit-Change-Number: 7793
Gerrit-PatchSet: 14
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Anonymous Coward #345
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:09:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Zach Amsden (Code Review)
Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1:

Sounds like https://jira.cloudera.com/browse/RELENG-2864


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:23:59 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread John Russell (Code Review)
John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8511/2/impala-docs.html
File impala-docs.html:

http://gerrit.cloudera.org:8080/#/c/8511/2/impala-docs.html@131
PS2, Line 131: HTML 
Documentation for Impala 2.10
 : PDF 
Documentation for Impala 2.10
> Nit: indent two spaces to the left, as these items are part of the same uno
Done. Ah, I see it was because ll 126-127 have consecutive  tags with the 
same indentation.I'll give an extra indent to ll 127-133 so lines can be copied 
between "latest release" and "older releases" without changing indentation.



--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:27:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs

2017-11-13 Thread Laszlo Gaal (Code Review)
Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs
..


Patch Set 3:

(3 comments)

> I also don't want to block progress on making this good improvement
 > to our S3 development, but my fear is that this script will get
 > more and more out of control if we don't draw a line somewhere
 > about what belongs in it.

That's a valid point. I moved the logic to bin/check-s3-access.sh, which 
actually lets us reuse the the same check elsewhere if it becomes necessary or 
useful.

http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@294
PS2, Line 294: export LOCAL_FS="file:${WAREHOUSE_LOCATION_PREFIX}"
> Yeah, I'd question whether "aws ls" belongs in this script either.
Done, moved to bin/check-s3-access.sh.


http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@303
PS2, Line 303: set AWS_A
> This variable will leak out into the user's shell.
Done, I moved these checks into a separate script, check-s3-access.sh


http://gerrit.cloudera.org:8080/#/c/8294/2/bin/impala-config.sh@307
PS2, Line 307:
> Good point; I'll check if wget can be set up the same way.
Done. Although the check was moved to bin/check-s3-access.sh, I have replaced 
curl with wget, using equivalent parameters for silencing and short timeouts.



--
To view, visit http://gerrit.cloudera.org:8080/8294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:26:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread John Russell (Code Review)
Hello Bharath Vissapragada, Michael Brown, Laurel Hale,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8511

to look at the new patch set (#3).

Change subject: Update Impala docs for 2.10 release
..

Update Impala docs for 2.10 release

Commit hash = 40ec6d0080638efaf3260672ab54ea4674896c5e

Several new HTML files for this release.

Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
---
M docs/build/html/index.html
M docs/build/html/topics/impala_adls.html
M docs/build/html/topics/impala_alter_table.html
M docs/build/html/topics/impala_array.html
A docs/build/html/topics/impala_buffer_pool_limit.html
M docs/build/html/topics/impala_compute_stats.html
M docs/build/html/topics/impala_config_options.html
M docs/build/html/topics/impala_connecting.html
M docs/build/html/topics/impala_create_table.html
M docs/build/html/topics/impala_datetime_functions.html
A docs/build/html/topics/impala_default_spillable_buffer_size.html
M docs/build/html/topics/impala_exec_single_node_rows_threshold.html
M docs/build/html/topics/impala_fixed_issues.html
M docs/build/html/topics/impala_hbase.html
M docs/build/html/topics/impala_hints.html
M docs/build/html/topics/impala_incompatible_changes.html
M docs/build/html/topics/impala_joins.html
M docs/build/html/topics/impala_known_issues.html
M docs/build/html/topics/impala_kudu.html
M docs/build/html/topics/impala_logging.html
M docs/build/html/topics/impala_map.html
M docs/build/html/topics/impala_math_functions.html
A docs/build/html/topics/impala_max_row_size.html
A docs/build/html/topics/impala_min_spillable_buffer_size.html
M docs/build/html/topics/impala_new_features.html
M docs/build/html/topics/impala_partitioning.html
M docs/build/html/topics/impala_perf_joins.html
M docs/build/html/topics/impala_perf_stats.html
M docs/build/html/topics/impala_query_options.html
M docs/build/html/topics/impala_reserved_words.html
M docs/build/html/topics/impala_scalability.html
M docs/build/html/topics/impala_select.html
M docs/build/html/topics/impala_shell_commands.html
M docs/build/html/topics/impala_shell_running_commands.html
M docs/build/html/topics/impala_ssl.html
M docs/build/html/topics/impala_string_functions.html
M docs/build/html/topics/impala_struct.html
M docs/build/html/topics/impala_subqueries.html
A docs/build/html/topics/impala_tablesample.html
M docs/build/html/topics/impala_upgrading.html
M docs/build/html/topics/impala_views.html
A docs/build/impala-2.10.pdf
M impala-docs.html
43 files changed, 3,570 insertions(+), 1,118 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/8511/3
--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] Remove unused/defunct Maven repositories.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8497 )

Change subject: Remove unused/defunct Maven repositories.
..


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1461/


--
To view, visit http://gerrit.cloudera.org:8080/8497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79eb6c483561726c7cbaf86874001f1979128720
Gerrit-Change-Number: 8497
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:33:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] Expose $IMPALA MAVEN OPTIONS for configuring Maven.

2017-11-13 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8496 )

Change subject: Expose $IMPALA_MAVEN_OPTIONS for configuring Maven.
..


Patch Set 4: Code-Review+2

Rebase, carry + 2


--
To view, visit http://gerrit.cloudera.org:8080/8496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c62185476fd2388c7cda8884276b79a77370127
Gerrit-Change-Number: 8496
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:58:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] Expose $IMPALA MAVEN OPTIONS for configuring Maven.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8496 )

Change subject: Expose $IMPALA_MAVEN_OPTIONS for configuring Maven.
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1462/


--
To view, visit http://gerrit.cloudera.org:8080/8496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c62185476fd2388c7cda8884276b79a77370127
Gerrit-Change-Number: 8496
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Nov 2017 21:59:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs

2017-11-13 Thread Laszlo Gaal (Code Review)
Hello Lars Volker, Michael Brown, Jim Apple, Philip Zeyliger, Sailesh Mukil, 
David Knupp, Joe McDonnell, Tim Armstrong, Alex Behm,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8294

to look at the new patch set (#3).

Change subject: IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs
..

IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs

For some time Impala in a production environment has been able
to access data stored in Amazon S3 buckets using credentials specified
in a number of ways:
- storing Amazon access keys in environment variables or
  in core-site.xml.
- using proprietary management tools to store Amazon access keys
  securely
- using Amazon IAM roles bound to VMs running in EC2.

The development minicluster environment used the first approach,
which risked leaking these keys.

This change enables Impala builds to use IAM
roles to access S3 buckets when running on an Amazon EC2 virtual
machine. The changes mainly ensure that environment variables and/or Jenkins
parameters carrying the traditional AWS credentials do not conflict with
credentials supplied by the IAM role attached to the VM instance.

The change also moves the logic performing the S3 access checks into a separate
script file: bin/check-s3-access.sh.

IAM role based credentials are accessible through the EC2
instance-property mechanism; for further details see Amazon's docs at
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials

Changes to the configuration script:
1. bin/impala-config.sh stops setting the AWS_* environment variables
   to dummy default values. When AWS credentials are not supplied in
   the environment variables AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY,
   these variables are unset (removed from the environment), otherwise
   they would interfere with authentication based on the IAM role.
2. Having AWS credentials in the AWS_* environment variables is now
   optional. They are still accepted to allow for private test runs
   accessing private/nondefault buckets with custom credentials.
3. bin/impala-config.sh now calls bin/check-s3-access.sh to perform the actual
   S3-dependent checks. check-s3-access.sh contains the S3-specific logic and
   network access needed to check if the requested S3 bucket is accessible
   for the build.

Changes to the minicluster configuration:
1. Security credentials for the s3n: connector, located in core-site.xml
   are no longer replaced with actual AWS_ credentials when configuring
   the minicluster. These parameters are used for some front-end tests,
   which don't actually reach out to S3, the s3n: notation just simulates
   non-HDFS storage.
   For these tests to work s3n: authentication parameters still need to
   exist in core-site.xml. Their values do not matter, so the configuration
   template now has fixed dummy values for these parameters.

2. Remove empty s3a: security parameter sections from core-site.xml:
   The testdata/cluster/admin setup script substitutes values from
   environment variables into core-site.xml when it sets up the minicluster
   runtime environment.

   The configuration section for s3a: credentials is now completely
   removed if both of the following conditions are met:
   - the target filesystem is set to "s3"
   - the AWS credential environment variables AWS_ACCESS_KEY_ID
 and AWS_SECRET_ACCESS_KEY are both empty or missing.

   The configuration file core-site.xml.tmpl is extended with
   comment markers that delimit the section to be removed in this case.

Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
---
A bin/check-s3-access.sh
M bin/impala-config.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.tmpl
4 files changed, 163 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/8294/3
--
To view, visit http://gerrit.cloudera.org:8080/8294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1460/


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:13:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] Pin gen build version's git handling to typical git dir.

2017-11-13 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8500 )

Change subject: Pin gen_build_version's git handling to typical git dir.
..


Patch Set 1:

> Sounds like https://jira.cloudera.com/browse/RELENG-2864

Zach, this is the public ASF Impala repo. That JIRA is not visible to all 
Impala contributors, only those who work at Cloudera.

Apache Impala should be as inclusive as possible, and not work in public on 
patches in a way only Clouderans can view.

Thanks!


--
To view, visit http://gerrit.cloudera.org:8080/8500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I98870a1cb2073ddcf9ba1620e3801e1964930edd
Gerrit-Change-Number: 8500
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:31:10 +
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Update Impala docs for 2.10 release

2017-11-13 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8511 )

Change subject: Update Impala docs for 2.10 release
..


Patch Set 3: Code-Review+2

Thank you for doing this!


--
To view, visit http://gerrit.cloudera.org:8080/8511
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Idea1f417d404b99dad9fb8255849867f1e67e767
Gerrit-Change-Number: 8511
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:39:14 +
Gerrit-HasComments: No