[Impala-ASF-CR] IMPALA-7067: deflake test cancellation

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10499 )

Change subject: IMPALA-7067: deflake test_cancellation
..


Patch Set 1:

Running tests to make sure that it works.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c90d4f5c277f7b0d5561637944b454f7a44c76e
Gerrit-Change-Number: 10499
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 04:59:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 3:

Hit IMPALA-7067. Will hold off on merging in case this patch increased the 
changes of hitting that flakiness.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 04:59:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7067: deflake test cancellation

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10499 )

Change subject: IMPALA-7067: deflake test_cancellation
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c90d4f5c277f7b0d5561637944b454f7a44c76e
Gerrit-Change-Number: 10499
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 24 May 2018 04:59:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 04:20:33 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Hello Joe McDonnell, Tim Armstrong, Impala Public Jenkins,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..

Trimming build-all-flag-combinations and adding minicluster profile.

[For the 2.x branch cherrypick, this change removes the "profile" stuff.]

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 7 builds, exercising every path,
but not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 3
  Options -skiptests -noclean -so and profile 2
  Options -skiptests -noclean -release and profile 3
  Options -skiptests -noclean -release -so -ninja and profile 3
  Options -skiptests -noclean -asan and profile 3
  Options -skiptests -noclean -ubsan -so -ninja and profile 2
  Options -skiptests -noclean -tsan and profile 3

I've also added explicitly building with both minicluster profiles
and a trivial dryrun option to print the above.

The options are simply hard-coded rather than being clever
and generating them. We decided this was easier to deal with.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Reviewed-on: http://gerrit.cloudera.org:8080/10489
Reviewed-by: Joe McDonnell 
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 41 insertions(+), 30 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10497
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR](2.x) Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10497 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10497
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 04:06:30 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-6131: Track time of last statistics update in metadata

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10484 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..

IMPALA-6131: Track time of last statistics update in metadata

The timestamp of the last COMPUTE STATS operation is saved to
table property "impala.lastComputeStatsTime". The format is
the same as in "transient_lastDdlTime", so the two can be
compared to check if the schema has changed since computing
statistics.

Other changes:
- Handling of "transient_lastDdlTime" is simplified - the old
  logic set it to current time + 1, if the old version was
  >= current time, to ensure that it is always increased by
  DDL operations. This was useful in the past, as IMPALA-387
  used lastDdlTime to check if partition data needs to be
  reloaded, but since IMPALA-1480, Impala does not rely on
  lastDdlTime at all.

- Computing / setting stats on HDFS tables no longer increases
  "transient_lastDdlTime".

- When Kudu tables are (re)loaded, it is checked if their
  HMS representation is up to date, and if it is, then
  IMetaStoreClient.alter_table() is not called. The old
  logic always called alter_table() after loading metadata
  from Kudu. This change was needed to ensure that
  "transient_lastDdlTime" works similarly in HDFS and Kudu
  tables, and should also make (re)loading Kudu tables faster.

Notes:
- Kudu will be able to sync its tables to HMS in the near
  future (see KUDU-2191), so the Kudu metadata handling in
  Impala may need to be redesigned.

Testing:
tests/metadata/test_last_ddl_time_update.py is extended by
- also checking "impala.lastComputeStatsTime"
- testing more SQL statements
- tests for Kudu tables

Note that test_last_ddl_time_update.py is ran only in
exhaustive testing.

Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Reviewed-on: http://gerrit.cloudera.org:8080/10484
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.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
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/metadata/test_last_ddl_time_update.py
7 files changed, 226 insertions(+), 173 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Gerrit-Change-Number: 10484
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR](2.x) IMPALA-6131: Track time of last statistics update in metadata

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10484 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Gerrit-Change-Number: 10484
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 24 May 2018 03:59:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 03:54:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 03:54:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 3:

hit IMPALA-7058


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 03:54:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 03:44:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7044: Prevent overflow when computing Parquet block size

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10483 )

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10483/2/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/10483/2/be/src/exec/hdfs-parquet-table-writer.cc@789
PS2, Line 789: stringstream ss;
 : ss << "Minimum required block size is too large, try 
reducing the number of columns "
 : "in the target table.";
why use stringstream if you're not going to construct a string?
(though maybe it'd be good to include num_cols in the error message, but 
consider using Substitute() if you do that).
also, let's add a comment explaining where the signed 32-bit limit comes from.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Thu, 24 May 2018 03:24:54 +
Gerrit-HasComments: Yes


[native-toolchain-CR] Bump libunwind version to 1.3-rc1

2018-05-23 Thread Lars Volker (Code Review)
Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10496


Change subject: Bump libunwind version to 1.3-rc1
..

Bump libunwind version to 1.3-rc1

This changes bumps libunwind to version 1.3-rc1, the same version as in
Kudu. It also adds two patches to improve its interaction with asan and
tsan. The patch we had for 1.1 is not needed anymore.

I tested this on my local dev environment with a prototype that rebases
our KRPC dependencies.

Change-Id: Ic34b78fd3272c12eb529ff06c7f9f1569d5def98
---
M buildall.sh
M source/libunwind/build.sh
D 
source/libunwind/libunwind-1.1-patches/0001-Build-libunwind-1.1-on-ppc64le.patch
A 
source/libunwind/libunwind-1.3-rc1-patches/0001-libunwind-Use-syscall-directly-in-write_validate-to-avoid-ASAN.patch
A 
source/libunwind/libunwind-1.3-rc1-patches/0002-libunwind-trace-cache-destructor.patch
5 files changed, 78 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/96/10496/1
--
To view, visit http://gerrit.cloudera.org:8080/10496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic34b78fd3272c12eb529ff06c7f9f1569d5def98
Gerrit-Change-Number: 10496
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 


[Impala-ASF-CR] [DOCS] Added a link to impala kerberos doc in impala-shell options doc

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10482 )

Change subject: [DOCS] Added a link to impala kerberos doc in impala-shell 
options doc
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbd542dc0e9051e022636dd1b6bebb69b9b22b08
Gerrit-Change-Number: 10482
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 24 May 2018 01:43:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Added a link to impala kerberos doc in impala-shell options doc

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10482 )

Change subject: [DOCS] Added a link to impala kerberos doc in impala-shell 
options doc
..

[DOCS] Added a link to impala kerberos doc in impala-shell options doc

Change-Id: Ifbd542dc0e9051e022636dd1b6bebb69b9b22b08
Reviewed-on: http://gerrit.cloudera.org:8080/10482
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins 
---
M docs/topics/impala_shell_options.xml
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifbd542dc0e9051e022636dd1b6bebb69b9b22b08
Gerrit-Change-Number: 10482
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] [DOCS] Added a link to impala kerberos doc in impala-shell options doc

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10482 )

Change subject: [DOCS] Added a link to impala kerberos doc in impala-shell 
options doc
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbd542dc0e9051e022636dd1b6bebb69b9b22b08
Gerrit-Change-Number: 10482
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 24 May 2018 01:34:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4025: Part 3: Add percentile cont & median aggregation functions

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/9778 )

Change subject: IMPALA-4025: Part 3: Add percentile_cont & median aggregation 
functions
..

IMPALA-4025: Part 3: Add percentile_cont & median aggregation functions

percentile_cont is implemented in the similar way as percentile_disc,
except for using a BE custom aggregation function for interpolating the
final result. median is rewritten into percentile_cont(0.5).

Some EE tests are added. Tests not related to error handling are
verified against PostgreSQL.

Change-Id: I2cc184682bb1bf4a5011b69a89e9ae253f3fd88d
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/PercentileAggExpr.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/aggregation.test
11 files changed, 242 insertions(+), 40 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cc184682bb1bf4a5011b69a89e9ae253f3fd88d
Gerrit-Change-Number: 9778
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 


[Impala-ASF-CR] IMPALA-4025: Part 1: Generalize and cleanup StmtRewriter

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10495


Change subject: IMPALA-4025: Part 1: Generalize and cleanup StmtRewriter
..

IMPALA-4025: Part 1: Generalize and cleanup StmtRewriter

This patch generalizes StmtRewriter, allowing it to be subclassed. The
base class would traverse the stmt tree while the subclasses can install
hooks to execute specific rewrite rules at certain places. Existing
rewriting rules are moved into SubqueryRewriter.

Change-Id: I9e7a6108d3d49be12ae032fdb54b5c3c23152a47
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
2 files changed, 995 insertions(+), 984 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e7a6108d3d49be12ae032fdb54b5c3c23152a47
Gerrit-Change-Number: 10495
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 


[Impala-ASF-CR] IMPALA-4025: Part 2: Add percentile disc aggregation function

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#9). ( 
http://gerrit.cloudera.org:8080/9777 )

Change subject: IMPALA-4025: Part 2: Add percentile_disc aggregation function
..

IMPALA-4025: Part 2: Add percentile_disc aggregation function

This patch adds aggregation function percentile_disc. The implementation
is rewriting it into an inline view. The inline view computes the row
number on the ordering expr using analytic functions. The parent query
then picks the desired row using aggregation.
An Example of such rewrite is in StmtRewriter.java.

The behavior of this function is mostly the same as in PostgreSQL. The
handling of percentile expr not in [0, 1] is different: PostgreSQL
throws an error and impala returns NULL.

Some FE and EE tests are added. EE tests not related to the above
difference are verified against PostgreSQL.

Change-Id: Iacef7b3fcd74c4c73d88400ce27307c3baa0121e
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
A fe/src/main/java/org/apache/impala/analysis/PercentileAggExpr.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/aggregation.test
21 files changed, 1,220 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/9777/9
--
To view, visit http://gerrit.cloudera.org:8080/9777
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacef7b3fcd74c4c73d88400ce27307c3baa0121e
Gerrit-Change-Number: 9777
Gerrit-PatchSet: 9
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:52:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:52:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6953: part 1: clean up DiskIoMgr

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: part 1: clean up DiskIoMgr
..


Patch Set 11:

It's probably one of these java udf queries


06:40:15 [gw1] FAILED 
query_test/test_udfs.py::TestUdfTargeted::test_hive_udfs_missing_jar[exec_option:
 {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': False, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: text/none]
06:40:15 [gw6] FAILED 
query_test/test_udfs.py::TestUdfExecution::test_java_udfs[exec_option: 
{'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 
'exec_single_node_rows_threshold': 0, 'enable_expr_rewrites': True} | 
table_format: text/none]
06:40:15 [gw9] PASSED 
query_test/test_udfs.py::TestUdfTargeted::test_udf_update_via_drop[exec_option: 
{'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': False, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: text/none]
06:40:16 [gw8] PASSED 
query_test/test_udfs.py::TestUdfTargeted::test_concurrent_jar_drop_use[exec_option:
 {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': False, 'abort_on_error': 1, 'debug_action': None, 
'exec_single_node_rows_threshold': 0} | table_format: text/none]


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a6e393f8c01d10143cbac91108af37f6498c1b1
Gerrit-Change-Number: 10245
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:36:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6953: part 1: clean up DiskIoMgr

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: part 1: clean up DiskIoMgr
..


Patch Set 11:

The job hit a weird crash, need to investigate more:



Stack: [0x7f5508de7000,0x7f55095e8000],  sp=0x7f55095e2c30,  free 
space=8175k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x8a8107]
V  [libjvm.so+0x96cf5f]
v  ~RuntimeStub::_complete_monitor_locking_Java
J 993 C2 
java.util.concurrent.ConcurrentHashMap.putVal(Ljava/lang/Object;Ljava/lang/Object;Z)Ljava/lang/Object;
 (362 bytes) @ 0x7f55b6cf0960 [0x7f55b6ceff40+0xa20]
J 9078 C2 java.lang.ClassLoader.loadClass(Ljava/lang/String;Z)Ljava/lang/Class; 
(122 bytes) @ 0x7f55b812b8dc [0x7f55b812b820+0xbc]
J 11268 C2 
java.net.FactoryURLClassLoader.loadClass(Ljava/lang/String;Z)Ljava/lang/Class; 
(40 bytes) @ 0x7f55b6b7a770 [0x7f55b6b7a720+0x50]
J 8675 C2 java.lang.ClassLoader.loadClass(Ljava/lang/String;)Ljava/lang/Class; 
(7 bytes) @ 0x7f55b71e1e28 [0x7f55b71e1c20+0x208]
v  ~StubRoutines::call_stub
V  [libjvm.so+0x6648eb]
V  [libjvm.so+0x661ec4]
V  [libjvm.so+0x662523]
V  [libjvm.so+0x9e398d]
V  [libjvm.so+0x9e2326]
V  [libjvm.so+0x9e2b50]
V  [libjvm.so+0x42c099]
V  [libjvm.so+0x9dc786]
V  [libjvm.so+0x6a5edf]
V  [libjvm.so+0x6a70cb]  JVM_DefineClass+0xbb
V  [libjvm.so+0xa31ea5]
V  [libjvm.so+0xa37ea7]
J 11139  
sun.misc.Unsafe.defineClass(Ljava/lang/String;[BIILjava/lang/ClassLoader;Ljava/security/ProtectionDomain;)Ljava/lang/Class;
 (0 bytes) @ 0x7f55b872f94b [0x7f55b872f840+0x10b]
J 11743 C1 
sun.reflect.MethodAccessorGenerator$1.run()Lsun/reflect/MagicAccessorImpl; (41 
bytes) @ 0x7f55b88ace34 [0x7f55b88ac240+0xbf4]
J 11742 C1 sun.reflect.MethodAccessorGenerator$1.run()Ljava/lang/Object; (5 
bytes) @ 0x7f55b87de794 [0x7f55b87de700+0x94]
v  ~StubRoutines::call_stub
V  [libjvm.so+0x6648eb]
V  [libjvm.so+0x6b5949]  JVM_DoPrivileged+0x429
J 1151  
java.security.AccessController.doPrivileged(Ljava/security/PrivilegedAction;)Ljava/lang/Object;
 (0 bytes) @ 0x7f55b6d8a17f [0x7f55b6d8a0c0+0xbf]
J 11505 C1 
sun.reflect.MethodAccessorGenerator.generate(Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Class;Ljava/lang/Class;[Ljava/lang/Class;IZZLjava/lang/Class;)Lsun/reflect/MagicAccessorImpl;
 (762 bytes) @ 0x7f55b818acb4 [0x7f55b8185580+0x5734]
J 11326 C2 
sun.reflect.NativeMethodAccessorImpl.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
 (104 bytes) @ 0x7f55b87fcea8 [0x7f55b87fcac0+0x3e8]
J 10406 C2 org.apache.impala.hive.executor.UdfExecutor.evaluate()V (396 bytes) 
@ 0x7f55b7da5bfc [0x7f55b7da59c0+0x23c]
v  ~StubRoutines::call_stub
V  [libjvm.so+0x6648eb]
V  [libjvm.so+0x6822d7]
V  [libjvm.so+0x6862c9]
C  [impalad+0x2a00ce2]  JNIEnv_::CallNonvirtualVoidMethodA(_jobject*, _jclass*, 
_jmethodID*, jvalue const*)+0x40
C  [impalad+0x29fece7]  
impala::HiveUdfCall::Evaluate(impala::ScalarExprEvaluator*, impala::TupleRow 
const*) const+0x44b
C  [impalad+0x2a00a19]  
impala::HiveUdfCall::GetStringVal(impala::ScalarExprEvaluator*, 
impala::TupleRow const*) const+0xbb
C  [impalad+0x2a09e94]  
impala::ScalarExprEvaluator::GetValue(impala::ScalarExpr const&, 
impala::TupleRow const*)+0x36e
C  [impalad+0x2a49699]  
impala::ScalarFnCall::EvaluateNonConstantChildren(impala::ScalarExprEvaluator*, 
impala::TupleRow const*) const+0x9d
C  [impalad+0x2a4b3a2]  impala_udf::BooleanVal 
impala::ScalarFnCall::InterpretEval(impala::ScalarExprEvaluator*,
 impala::TupleRow const*) const+0x18c
C  [impalad+0x2a49865]  
impala::ScalarFnCall::GetBooleanVal(impala::ScalarExprEvaluator*, 
impala::TupleRow const*) const+0x179
C  [impalad+0x2a0a467]  
impala::ScalarExprEvaluator::GetBooleanVal(impala::TupleRow*)+0x37
C  [impalad+0x1b715af]  
impala::ExecNode::EvalPredicate(impala::ScalarExprEvaluator*, 
impala::TupleRow*)+0x23
C  [impalad+0x1b704a4]  
impala::ExecNode::EvalConjuncts(impala::ScalarExprEvaluator* const*, int, 
impala::TupleRow*)+0x42
C  [impalad+0x1bb682d]  
impala::HdfsScanner::EvalConjuncts(impala::TupleRow*)+0x4d
C  [impalad+0x1bb1047]  
impala::HdfsScanner::WriteCompleteTuple(impala::MemPool*, 
impala::FieldLocation*, impala::Tuple*, impala::TupleRow*, impala::Tuple*, 
unsigned char*, unsigned char*)+0x1b1
C  [impalad+0x1bc1eb7]  
impala::HdfsScanner::WriteAlignedTuples(impala::MemPool*, impala::TupleRow*, 
impala::FieldLocation*, int, int, int, int, bool)+0x1f3
C  [impalad+0x1bf9b23]  impala::HdfsTextScanner::WriteFields(int, int, 
impala::MemPool*, impala::TupleRow*)+0x779
C  [impalad+0x1bf5008]  
impala::HdfsTextScanner::ProcessRange(impala::RowBatch*, int*)+0x9d8
C  [impalad+0x1bf5d32]  
impala::HdfsTextScanner::GetNextInternal(impala::RowBatch*)+0x422
C  [impalad+0x1bafdc0]  impala::HdfsScanner::ProcessSplit()+0x1ea
C  [impalad+0x1b87028]  

[Impala-ASF-CR] IMPALA-6953: part 2: clean up DiskIoMgr

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10479 )

Change subject: IMPALA-6953: part 2: clean up DiskIoMgr
..


Patch Set 3: Code-Review+2

Fix a clang-tidy error. Need to investigate the other build failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie82849652266b6660dd479689e5134273b172eb5
Gerrit-Change-Number: 10479
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:32:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6953: part 2: clean up DiskIoMgr

2018-05-23 Thread Tim Armstrong (Code Review)
Hello Dan Hecht, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6953: part 2: clean up DiskIoMgr
..

IMPALA-6953: part 2: clean up DiskIoMgr

Follow-on changes.

Made further mechanical changes that involved a lot of code motion:
* Move code that manipulates RequestContext internal state to
  RequestContext
* Don't use protected friend pattern that doesn't work as I thought
* Remove some unused member variables and arguments
* Move PerDiskState definition into .cc

Change-Id: Ie82849652266b6660dd479689e5134273b172eb5
---
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/scanner-context.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
17 files changed, 527 insertions(+), 565 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie82849652266b6660dd479689e5134273b172eb5
Gerrit-Change-Number: 10479
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:30:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..

Trimming build-all-flag-combinations and adding minicluster profile.

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 7 builds, exercising every path,
but not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 3
  Options -skiptests -noclean -so and profile 2
  Options -skiptests -noclean -release and profile 3
  Options -skiptests -noclean -release -so -ninja and profile 3
  Options -skiptests -noclean -asan and profile 3
  Options -skiptests -noclean -ubsan -so -ninja and profile 2
  Options -skiptests -noclean -tsan and profile 3

I've also added explicitly building with both minicluster profiles
and a trivial dryrun option to print the above.

The options are simply hard-coded rather than being clever
and generating them. We decided this was easier to deal with.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Reviewed-on: http://gerrit.cloudera.org:8080/10489
Reviewed-by: Joe McDonnell 
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 50 insertions(+), 30 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, but someone else must approve
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
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] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
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: Thu, 24 May 2018 00:30:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:29:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 1:

(2 comments)

Maybe take another look to make sure that my change was what you intended

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

http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG@12
PS1, Line 12: Updating backend_exec_complete_barrier_ isn't actually necessary
: when handling error
> BTW, that's kind of true, except that if we've already seen an error then w
Yeah you're right, it is necessary if the state was already RETURNED_RESULTS 
and we got an error after that transition.


http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc@688
PS1, Line 688: if (VLOG_QUERY_IS_ON && pending_backends >= 0) {
 :   VLOG_QUERY << "Backend completed:"
 :  << " host=" << 
TNetworkAddressToString(backend_state->impalad_address())
 :  << " remaining=" << pending_backends
 :  << " query_id=" << PrintId(query_id());
 :   BackendState::LogFirstInProgress(backend_states_);
 : }
> I think I messed this up because I wanted this log to print before the logg
Yeah that makes sense actually, put it back up the top.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:28:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Hello Bikramjeet Vig, Dan Hecht,

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

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

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

Change subject: IMPALA-7055: fix race with DML errors
..

IMPALA-7055: fix race with DML errors

Error statuses could be lost because backend_exec_complete_barrier_
went to 0 before the query was transitioned to an error state.
Reordering the UpdateExecState() and backend_exec_complete_barrier_
calls prevents this race.

Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 19 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] [DOCS] Sentry is required for Impala to enable delegation

2018-05-23 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10451 )

Change subject: [DOCS] Sentry is required for Impala to enable delegation
..


Patch Set 2:

Is there a JIRA that states that delegation cannot be used without Sentry? I'm 
just looking to read more about the details before I sign off.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I002d3d33eee6a9b9336f21c81a4de75ed3bd5efb
Gerrit-Change-Number: 10451
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 24 May 2018 00:27:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4025: Part 2: Add percentile disc aggregation function

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/9777 )

Change subject: IMPALA-4025: Part 2: Add percentile_disc aggregation function
..

IMPALA-4025: Part 2: Add percentile_disc aggregation function

This patch adds aggregation function percentile_disc. The implementation
is rewriting it into an inline view. The inline view computes the row
number on the ordering expr using analytic functions. The parent query
then picks the desired row using aggregation.
An Example of such rewrite is in StmtRewriter.java.

The behavior of this function is mostly the same as in PostgreSQL. The
handling of percentile expr not in [0, 1] is different: PostgreSQL
throws an error and impala returns NULL.

Some FE and EE tests are added. EE tests not related to the above
difference are verified against PostgreSQL.

Change-Id: Iacef7b3fcd74c4c73d88400ce27307c3baa0121e
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
A fe/src/main/java/org/apache/impala/analysis/PercentileAggExpr.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/catalog/AggregateFunction.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/aggregation.test
21 files changed, 1,220 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/9777/8
--
To view, visit http://gerrit.cloudera.org:8080/9777
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacef7b3fcd74c4c73d88400ce27307c3baa0121e
Gerrit-Change-Number: 9777
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] [DOCS] Added a link to impala kerberos doc in impala-shell options doc

2018-05-23 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10482 )

Change subject: [DOCS] Added a link to impala kerberos doc in impala-shell 
options doc
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbd542dc0e9051e022636dd1b6bebb69b9b22b08
Gerrit-Change-Number: 10482
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 24 May 2018 00:25:47 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-6131: Track time of last statistics update in metadata

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10484 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Gerrit-Change-Number: 10484
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 24 May 2018 00:22:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:18:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 16: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 24 May 2018 00:13:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG@12
PS1, Line 12: Updating backend_exec_complete_barrier_ isn't actually necessary
: when handling error
BTW, that's kind of true, except that if we've already seen an error then we 
won't transition. So, handling an error here doesn't guarantee the notify with 
happen.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:04:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 1: Code-Review+2

(1 comment)

Thanks for taking care of that Tim. It looks right.

http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc@688
PS1, Line 688: if (VLOG_QUERY_IS_ON && pending_backends >= 0) {
 :   VLOG_QUERY << "Backend completed:"
 :  << " host=" << 
TNetworkAddressToString(backend_state->impalad_address())
 :  << " remaining=" << pending_backends
 :  << " query_id=" << PrintId(query_id());
 :   BackendState::LogFirstInProgress(backend_states_);
 : }
I think I messed this up because I wanted this log to print before the logging 
in UpdateExecState() (so it'd be clearer where the error comes from we are 
updating the exec state). Maybe the order isn't actually helpful, but if you 
think it does then an option is to use 
backend_exec_complete_barrier_->pending() to get pending_backends earlier 
without notifying until later.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 24 May 2018 00:02:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7061: Rework HBase splitting and assignment

2018-05-23 Thread Joe McDonnell (Code Review)
Hello Philip Zeyliger, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7061: Rework HBase splitting and assignment
..

IMPALA-7061: Rework HBase splitting and assignment

Some frontend PlannerTests rely on HBase tables being
arranged in a deterministic way. Specifically, the
HBase tables need to be split with specific region
boundaries and those regions need to be assigned to
specific HBase region servers.

Currently, the tables are created without splits and
testdata/bin/split-hbase.sh runs Java code in
HBaseTestDataRegionAssignment to split and assign
the tables. This runs during dataload via
testdata/bin/create-load-data.sh and during tests
with bin/run-all-tests.sh. There are problems with
both parts of this process. The table splitting is
flaky. Since significant time can pass between the
assignments and the tests, rebalancing means the
assignments are not always stable.

This changes the process so that the HBase tables are
created with the splits already specified via the
HBase shell. The splits remain stable over time.
PlannerTestBase runs the assignment code in
HBaseTestDataRegionAssignment at the start of
the PlannerTests. This makes the assignments
deterministic. No other tests depends on the
exact assignments, so this does not regress anything.

Testing:
 - Local testing
 - Ran gerrit-verify-dryrun-external
 - Verified minicluster profile 2 compiles

Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
---
M bin/run-all-tests.sh
A 
fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
A 
fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/bin/create-load-data.sh
M testdata/bin/generate-schema-statements.py
D testdata/bin/split-hbase.sh
M testdata/datasets/functional/functional_schema_template.sql
D 
testdata/src/compat-minicluster-profile-2/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssigment.java
D 
testdata/src/compat-minicluster-profile-3/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssigment.java
10 files changed, 329 insertions(+), 728 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
Gerrit-Change-Number: 10447
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-7061: Rework HBase splitting and assignment

2018-05-23 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10447 )

Change subject: IMPALA-7061: Rework HBase splitting and assignment
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
File 
fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java:

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@48
PS6, Line 48:  * Splits HBase tables into regions and deterministically assigns 
regions to region
> Gerrit is lame about not detecting the rename here and showing a useful dif
Updated the comments. This is only removing the splitting code. It is otherwise 
almost identical.


http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@142
PS6, Line 142:   public static String printKey(byte[] key) {
> Remove this and use fe/src/main/java/org/apache/impala/planner/HBaseScanNod
Done


http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
File 
fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java:

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@168
PS6, Line 168:   public static String printKey(byte[] key) {
> same here; we have another copy of this.
Done


http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql@144
PS6, Line 144: True
> Instead of this being a boolean, do you want to put the splits values here?
Switched this to be HBASE_REGION_SPLITS and specified the splits directly.


http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql@521
PS6, Line 521: True
> same
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
Gerrit-Change-Number: 10447
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 23 May 2018 23:36:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h
File be/src/util/runtime-profile.h:

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h@139
PS1, Line 139:   /// Sorts all children according to descending total time. 
Does not
> The class comment does say that it's thread-safe, so this was implied. Mayb
Good point, let's remove them.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 23:26:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h
File be/src/util/runtime-profile.h:

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h@139
PS1, Line 139:   /// Sorts all children according to descending total time. 
Does not
> Should we mention that this is now thread safe, similar to L119?
The class comment does say that it's thread-safe, so this was implied. Maybe I 
should remove the comment above, since it does imply that other functions might 
not be thread-safe.


http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.cc@517
PS1, Line 517: Reverse ordering: we want the longest first
> This comment could now be removed since L514 captures the intent already.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 22:36:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Tim Armstrong (Code Review)
Hello Lars Volker,

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

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

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

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..

IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

The API is inherently unsafe, because std::sort() can crash if a
comparator returns unstable results, e.g. because a profile
counter value changes. The generality of this function is both
unnecessary and dangerous.

The fix is to take a snapshot of the counter values and sort
based on the snapshot.

Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
4 files changed, 30 insertions(+), 26 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 


[Impala-ASF-CR] PREVIEW: IMPALA-110 (part 1): Refactor PartitionedAggregationNode

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10394 )

Change subject: PREVIEW: IMPALA-110 (part 1): Refactor 
PartitionedAggregationNode
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/non-grouping-aggregator.cc
File be/src/exec/non-grouping-aggregator.cc:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/non-grouping-aggregator.cc@43
PS1, Line 43: true
> why initialized to true?
I feel like we should defer changing some of the more subtle details of the 
logic until a follow-on patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e
Gerrit-Change-Number: 10394
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 23 May 2018 22:32:28 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] PREVIEW: IMPALA-110 (part 1): Refactor PartitionedAggregationNode

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10394 )

Change subject: PREVIEW: IMPALA-110 (part 1): Refactor 
PartitionedAggregationNode
..


Patch Set 1:

(7 comments)

I did a quick pass, didn't look closely for correctness issues. This seems like 
a vast improvement. I had some minor comments.

It seems like we could defer some of the cleanup to a follow-on patch if that 
makes this initial refactoring more mechanical (it gets harder to review if 
there's a mix of large-scale code motion and non-mechanical cleanup in the same 
file).

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregation-node.h
File be/src/exec/aggregation-node.h:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregation-node.h@38
PS1, Line 38:   virtual Status Init(const TPlanNode& tnode, RuntimeState* 
state);
Add override specifiers?


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregation-node.cc
File be/src/exec/aggregation-node.cc:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregation-node.cc@89
PS1, Line 89: if (UNLIKELY(VLOG_ROW_IS_ON)) {
I'd be ok with deleting this code :)


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregation-node.cc@111
PS1, Line 111: 
RETURN_IF_ERROR(aggregator_->MoveHashPartitions(child(0)->rows_returned()));
It feels like this is the wrong abstraction - should it just be InputDone() 
like the Sorter?


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregator.h
File be/src/exec/aggregator.h:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/aggregator.h@141
PS1, Line 141: int64_t num_rows_returned_;
 :   Runti
> why are both of these needed?
Yeah it seems like rows_returned_counter_ is probably only needed in ExecNode? 
Or is this looking forward to having a separate sub-profile for the multiple 
aggregators in a multi-agg node?


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/exec-node.cc@362
PS1, Line 362:   if (tnode.agg_node.use_streaming_preaggregation) {
Maybe these should be different TPlanNodeTypes now?


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/grouping-aggregator.h
File be/src/exec/grouping-aggregator.h:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/grouping-aggregator.h@117
PS1, Line 117:   virtual Status Init(const TPlanNode& tnode, RuntimeState* 
state);
Add overrides?


http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/non-grouping-aggregator.h
File be/src/exec/non-grouping-aggregator.h:

http://gerrit.cloudera.org:8080/#/c/10394/1/be/src/exec/non-grouping-aggregator.h@48
PS1, Line 48:   virtual Status Open(RuntimeState* state);
override specifiers?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e
Gerrit-Change-Number: 10394
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 23 May 2018 22:30:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function

2018-05-23 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10488 )

Change subject: IMPALA-7062: fix unsafe RuntimeProfile::SortChildren() function
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h
File be/src/util/runtime-profile.h:

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.h@139
PS1, Line 139:   /// Sorts all children according to descending total time. 
Does not
Should we mention that this is now thread safe, similar to L119?


http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/10488/1/be/src/util/runtime-profile.cc@517
PS1, Line 517: Reverse ordering: we want the longest first
This comment could now be removed since L514 captures the intent already.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie456c87c34e7d75eb7482f499e90d50de740df40
Gerrit-Change-Number: 10488
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 23 May 2018 22:27:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5168: Codegen HASH PARTITIONED KrpcDataStreamSender::Send()

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10421 )

Change subject: IMPALA-5168: Codegen HASH_PARTITIONED 
KrpcDataStreamSender::Send()
..


Patch Set 2:

Oh, that's not nearly as bad as I expected. I think we can probably live with 
that given the huge speedups we're getting elsewhere.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff
Gerrit-Change-Number: 10421
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 22:18:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5168: Codegen HASH PARTITIONED KrpcDataStreamSender::Send()

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10421 )

Change subject: IMPALA-5168: Codegen HASH_PARTITIONED 
KrpcDataStreamSender::Send()
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff
Gerrit-Change-Number: 10421
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 22:18:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10491


Change subject: IMPALA-7055: fix race with DML errors
..

IMPALA-7055: fix race with DML errors

Error statuses could be lost because backend_exec_complete_barrier_
went to 0 before the query was transitioned to an error state.

Updating backend_exec_complete_barrier_ isn't actually necessary
when handling error - UpdateExecState() already signals the barrier
when transitioning to an ERROR/CANCELLED terminal state.

Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 15 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7055: fix race with DML errors

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10491 )

Change subject: IMPALA-7055: fix race with DML errors
..


Patch Set 1:

I'm still in the process of testing this but wanted to get feedback on whether 
it's the correct fix.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2
Gerrit-Change-Number: 10491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 22:14:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 23 May 2018 21:45:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..

IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

This commit shims out HDFS's FileStatus.isErasureCoded() to manage
working with multiple versions of Hadoop.

I tested compilation with both profiles.

Cherry-picks: not for 2.x.

Change-Id: I423087078f84b0806545322519f224d58815123d
Reviewed-on: http://gerrit.cloudera.org:8080/10487
Reviewed-by: Philip Zeyliger 
Tested-by: Impala Public Jenkins 
---
A 
fe/src/compat-minicluster-profile-2/java/org/apache/impala/compat/HdfsShim.java
A 
fe/src/compat-minicluster-profile-3/java/org/apache/impala/compat/HdfsShim.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
3 files changed, 64 insertions(+), 2 deletions(-)

Approvals:
  Philip Zeyliger: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
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: Wed, 23 May 2018 21:03:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 21:02:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 4:

> This makes sense to me. Thanks for taking this on. These combinations will 
> make ccache worthless on this job,
> but I think that is actually a good indication that we aren't doing duplicate 
> stuff.

Yep, agree.

> What do you plan to have this script do on the 2.x branch?

Hadn't thought about it, but probably the same thing without the "profile" 
business. The funny thing is that the script will work just fine, because the 
extra environment variable will get ignored, but I'll clean it up.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 20:58:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 4: Code-Review+1

This makes sense to me. Thanks for taking this on. These combinations will make 
ccache worthless on this job, but I think that is actually a good indication 
that we aren't doing duplicate stuff.

What do you plan to have this script do on the 2.x branch?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 20:56:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

2018-05-23 Thread Vuk Ercegovac (Code Review)
Hello Lars Volker, Tianyi Wang, Dimitris Tsirogiannis, Alex Behm, Mostafa 
Mokhtar, Dan Hecht,

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

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

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

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..

IMPALA-5931: Generates scan ranges in planner for s3/adls

Currently, for filesystems that do not include physical
block information (e.g., block replica locations, caching),
synthetic blocks are generated and stored in the catalog
when metadata is loaded. Example file systems for which this is done
includes S3, ADLS, and local fs.

This change avoids generating these blocks when metadata is loaded.
Instead, scan ranges are directly generated from such files by the
backend coordinator. Previously, all scan ranges were produced by
the planner in HDFSScanNode in the frontend. Now, those files without
block information are sent to the coordinator represented by a split
specification that determines how the coordinator will create scan ranges
to send to executors.

This change reduces the space needed in the catalog and reduces the
scan range data structures that are passed from the frontend to the
backend when planning and coordinating a query.
In addition a bug is avoided where non-splittable files were being
split anyways to support the query parameter that places a limit on
scan ranges.

Testing:
- added backend scheduler tests
- mixed-filesystems test covers tables/queries with multiple fs's.
- local fs tests cover the code paths in this change
- all core tests pass when configured with s3
- manually tried larger local filesystem tables (tpch) with multiple
  partitions and observed the same scan ranges.
  - TODO: adls testing

Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
---
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler-test.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/util/CMakeLists.txt
A be/src/util/flat_buffer.cc
A be/src/util/flat_buffer.h
M common/thrift/Frontend.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Planner.thrift
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/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/ScanNode.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
21 files changed, 676 insertions(+), 246 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/8523/16
--
To view, visit http://gerrit.cloudera.org:8080/8523
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 16
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

2018-05-23 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 15:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8523/15/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/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@783
PS15, Line 783:   boolean checkMissingDiskIds = 
FileSystemUtil.supportsStorageIds(partitionFs);
  :   boolean supportsBlocks = 
FileSystemUtil.supportsStorageIds(partitionFs);
> these are equivalent. how about getting rid of the checkMissingDiskIds, and
see reply to last comment for naming.


http://gerrit.cloudera.org:8080/#/c/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@812
PS15, Line 812: result.first
> was changing the polarity of this a bug fix?
yes. there was a bug here, as well as double-counting total_bytes from a prev. 
merge, and several npe's from that latest thrift refactor (java thrift list is 
null, not empty)


http://gerrit.cloudera.org:8080/#/c/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@914
PS15, Line 914: checkMissingDiskIds && !fileDesc.getIsEc()
> this was confusing because when I read the name "checkMissingDiskIds" I tho
renamed to: fsHasBlocks



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 15
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 23 May 2018 20:44:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7058: disable fuzz test for RC and Seq

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10485 )

Change subject: IMPALA-7058: disable fuzz test for RC and Seq
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 23 May 2018 20:25:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7058: disable fuzz test for RC and Seq

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10485 )

Change subject: IMPALA-7058: disable fuzz test for RC and Seq
..

IMPALA-7058: disable fuzz test for RC and Seq

There appear to still be some rare crashes. Let's disable the
test until we can sort those out

Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Reviewed-on: http://gerrit.cloudera.org:8080/10485
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 1 insertion(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 


[native-toolchain-CR] Upgrade Protobuf to 3.5.1

2018-05-23 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10480 )

Change subject: Upgrade Protobuf to 3.5.1
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10480/1/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/10480/1/buildall.sh@97
PS1, Line 97: if (( BUILD_HISTORICAL )) ; then
I think it might not be necessary to build old versions since we've switched to 
versioning each build.



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e4e9dc301ff16f9aecc32d30c22b523dda3033c
Gerrit-Change-Number: 10480
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 23 May 2018 20:15:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Hello Tim Armstrong, Joe McDonnell,

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

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

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

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..

Trimming build-all-flag-combinations and adding minicluster profile.

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 7 builds, exercising every path,
but not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 3
  Options -skiptests -noclean -so and profile 2
  Options -skiptests -noclean -release and profile 3
  Options -skiptests -noclean -release -so -ninja and profile 3
  Options -skiptests -noclean -asan and profile 3
  Options -skiptests -noclean -ubsan -so -ninja and profile 2
  Options -skiptests -noclean -tsan and profile 3

I've also added explicitly building with both minicluster profiles
and a trivial dryrun option to print the above.

The options are simply hard-coded rather than being clever
and generating them. We decided this was easier to deal with.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 50 insertions(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Hello Tim Armstrong, Joe McDonnell,

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

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

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

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..

Trimming build-all-flag-combinations and adding minicluster profile.

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 7 builds, exercising every path,
but not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 3
  Options -skiptests -noclean -so and profile 2
  Options -skiptests -noclean -release and profile 3
  Options -skiptests -noclean -release -so -ninja and profile 3
  Options -skiptests -noclean -asan and profile 3
  Options -skiptests -noclean -ubsan -so -ninja and profile 2
  Options -skiptests -noclean -tsan and profile 3

I've also added explicitly building with both minicluster profiles
and a trivial dryrun option to print the above.

The options are simply hard-coded rather than being clever
and generating them. We decided this was easier to deal with.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 50 insertions(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Trimming build-all-flag-combinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-combinations and adding minicluster 
profile.
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@7
PS1, Line 7: cobinations
> typo
Done


http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@20
PS1, Line 20:   Options -skiptests -noclean -release and profile 2
> I think hard-coding would be easier to understand when N=7. The current app
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 19:55:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7061: Rework HBase splitting and assignment

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10447 )

Change subject: IMPALA-7061: Rework HBase splitting and assignment
..


Patch Set 6:

(5 comments)

Thanks for tackling this. This looks great. I didn't manually look at the diff 
of HBaseTestDataRegionAssignment, but I assume you just removed the splitting 
stuff. If you did more interesting surgery, let me know, and I'll take a more 
careful look.

I think we can move the splitpoints into the template.sql file, but I'm ok 
either way.

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
File 
fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java:

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@48
PS6, Line 48:  * Splits HBase tables into regions and deterministically assigns 
regions to region
Gerrit is lame about not detecting the rename here and showing a useful diff. 
It looks like you removed the splitting part of the code. if so, update the 
comments?


http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@142
PS6, Line 142:   public static String printKey(byte[] key) {
Remove this and use 
fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java's version?


http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
File 
fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java:

http://gerrit.cloudera.org:8080/#/c/10447/6/fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java@168
PS6, Line 168:   public static String printKey(byte[] key) {
same here; we have another copy of this.


http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql@144
PS6, Line 144: True
Instead of this being a boolean, do you want to put the splits values here? It 
means that there will be two copies (for the two relevant table), but that 
seems correct.


http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/datasets/functional/functional_schema_template.sql@521
PS6, Line 521: True
same



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
Gerrit-Change-Number: 10447
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 23 May 2018 19:33:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@7
PS1, Line 7: cobinations
typo


http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@20
PS1, Line 20:   Options -skiptests -noclean -release and profile 2
> I added that.
I think hard-coding would be easier to understand when N=7. The current 
approach isn't terrible to understand but I don't think helps.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 19:31:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6909: [DOCS] SET ROW FORMAT in ALTER TABLE

2018-05-23 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10452 )

Change subject: IMPALA-6909: [DOCS] SET ROW FORMAT in ALTER TABLE
..


Patch Set 1:

Could you please review this change and approve?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib680f25d8c929e1194a2274777f83851b04bcb93
Gerrit-Change-Number: 10452
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 23 May 2018 19:23:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@20
PS1, Line 20:   Options -skiptests -noclean -release and profile 2
> I'd suggest include a DEBUG/-so and RELEASE/-so build combination, since th
I added that.

Take a look: Maybe the right answer is just to hard-code the 7 builds we're 
doing; I'm not sure I've saved any mental or code capacity with the clever 
iteration.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 19:15:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Hello Tim Armstrong, Joe McDonnell,

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

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

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

Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..

Trimming build-all-flag-cobinations and adding minicluster profile.

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 7 builds, exercising every path,
but not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 3
  Options -skiptests -noclean -so -ninja and profile 2
  Options -skiptests -noclean -release and profile 3
  Options -skiptests -noclean -release -so -ninja and profile 2
  Options -skiptests -noclean -asan and profile 3
  Options -skiptests -noclean -ubsan -so -ninja and profile 2
  Options -skiptests -noclean -tsan and profile 3

I've also added explicitly building with both minicluster profiles
and a trivial dryrun option to print the above.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 42 insertions(+), 29 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-23 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10060 )

Change subject: IMPALA-5216: Make admission control queuing async
..


Patch Set 14:

(1 comment)

Just rebased in patchset 14, will address comments in the next patchset

http://gerrit.cloudera.org:8080/#/c/10060/12/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

http://gerrit.cloudera.org:8080/#/c/10060/12/be/src/service/impala-http-handler.cc@776
PS12, Line 776:  request_state->operation_state() != 
TOperationState::ERROR_STATE
> Yeah the logic of this function is a bit weird. It looks like we actually r
@Tim You are right, anything not used/set is just returned as empty which is 
basically what you will get from the archived query. Hence removing this 
condition makes sense, will do so in the next patch



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 14
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 19:04:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7044: Prevent overflow when computing Parquet block size

2018-05-23 Thread Thomas Marshall (Code Review)
Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10483 )

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 23 May 2018 19:05:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-23 Thread Bikramjeet Vig (Code Review)
Hello Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-5216: Make admission control queuing async
..

IMPALA-5216: Make admission control queuing async

Implement asynchronous admission control queuing. This is achieved by
running the admission control code-path in a separate thread. Major
changes include: propagating cancellation to the admission control
thread and dequeuing thread, and adding a new Query Operation State
called "PENDING" that represents the state between completion of
planning and starting of query execution.

Testing:
- Added a deterministic end to end test
- Ran multiple stress tests successfully with a cancellation probability
of 60% and with different values for the following parameters:
max_requests, queue_wait_timeout_ms. Ensured that the impalad was in a
valid state afterwards (no orphan fragments or wrong metrics).
- Ran all exhaustive tests and ASAN core tests successfully.

Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
---
M be/src/common/atomic.h
M be/src/common/logging.h
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/promise-test.cc
M be/src/util/promise.h
M common/thrift/ImpalaService.thrift
M tests/authorization/test_authorization.py
M tests/beeswax/impala_beeswax.py
M tests/common/impala_connection.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_hs2.py
M tests/query_test/test_observability.py
M www/query_backends.tmpl
27 files changed, 607 insertions(+), 207 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/10060/14
--
To view, visit http://gerrit.cloudera.org:8080/10060
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 14
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10489/1//COMMIT_MSG@20
PS1, Line 20:   Options -skiptests -noclean -release and profile 2
I'd suggest include a DEBUG/-so and RELEASE/-so build combination, since there 
tend to be interactions between the compiler, build type and linking. At a 
minimum we should be testing GCC/-so, but I've seen cases before where debug 
builds work and release builds don't because some function was inlined in the 
release build and not in the debug build.

I think adding -so for those build types should be relatively cheap, since 
IMPALA-6974 means that we'll get ccache hits.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 19:02:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10489 )

Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..


Patch Set 1:

I've triggered https://jenkins.impala.io/job/all-build-options-ub1604/1712 
against this change by way of testing.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 23 May 2018 18:48:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] Trimming build-all-flag-cobinations and adding minicluster profile.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10489


Change subject: Trimming build-all-flag-cobinations and adding minicluster 
profile.
..

Trimming build-all-flag-cobinations and adding minicluster profile.

build-all-flag-combinations.sh is the long pole in pre-commit testing.
It takes about 2.5 hours and runs about 40 builds per the following.

  $ curl --silent 
https://jenkins.impala.io/job/all-build-options-ub1604/1707/consoleText | grep 
'Building with OPTIONS' | wc -l
  40

This commit changes this to run only 5 builds, exercising every path,
butbut not every combination of paths. The paths exercised are:

  Options -skiptests -noclean and profile 2
  Options -skiptests -noclean -asan -so -ninja and profile 3
  Options -skiptests -noclean -release and profile 2
  Options -skiptests -noclean -ubsan -so -ninja and profile 3
  Options -skiptests -noclean -tsan and profile 2

I've also added explicitly building with both minicluster profiles.

Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
---
M bin/jenkins/build-all-flag-combinations.sh
1 file changed, 32 insertions(+), 29 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I258732a3d963958f76d99f9d7b51450ed67bec21
Gerrit-Change-Number: 10489
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

(1 comment)

Is this a breaking change? We normally hold on braking changes until we bump 
the major version.

Is IMPALA-3307 going to go in on a minor release? Is it a breaking change?

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h
File be/src/exprs/timezone_db.h:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55
PS1, Line 55: map
> Maybe use unordered_map instead ? Probably wouldn't make much of a differen
const?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Wed, 23 May 2018 18:30:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@67
PS1, Line 67: import org.apache.impala.compat.HdfsShim;
> I think they are? common < compat < fb?
I misread



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 23 May 2018 18:25:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 23 May 2018 18:22:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..


Patch Set 2: Code-Review+2

Carrying +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 23 May 2018 18:22:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7044: Prevent overflow when computing Parquet block size

2018-05-23 Thread Lars Volker (Code Review)
Hello Thomas Marshall,

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

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

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

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
..

IMPALA-7044: Prevent overflow when computing Parquet block size

When writing Parquet files we compute a minimum block size based on the
number of columns in the target table:

  3 * page_size * num_cols

For tables with a large number of columns (> ~10k), this value will get
larger than 2GB. When we pass it to hdfsOpenFile() in
HdfsTableSink::CreateNewTmpFile() it gets cast to a signed int32 and can
overflow.

To fix this we return an error if we detect that the minimum block size
exceed 2GB.

This change adds a test using CTAS into a table with 12k columns, making
sure that Impala returns the correct error.

Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
---
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/hdfs-table-sink.cc
M tests/query_test/test_insert_parquet.py
4 files changed, 35 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7044: Prevent overflow when computing Parquet block size

2018-05-23 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10483 )

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-parquet-table-writer.cc@795
PS1, Line 795: table_desc_->num_cols() - table_desc_->num_clustering_cols()
> num_cols
Done


http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-table-sink.cc@376
PS1, Line 376: return Status("HDFS block size must be smaller than 2GB.");
> Any reasonable way to test this path?
I can't think of one, it's mainly another safe-guard against future mistakes. 
The parquet_file_size query option does the same check.

It is actually OK to allow 2GB block sizes to I loosened the check. Before 
doing so I was able to trigger this manually by setting parquet_file_size to 
int_max.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 23 May 2018 18:20:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Tianyi Wang (Code Review)
Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10487 )

Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..


Patch Set 1: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/compat-minicluster-profile-2/java/org/apache/impala/compat/HdfsShim.java
File 
fe/src/compat-minicluster-profile-2/java/org/apache/impala/compat/HdfsShim.java:

http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/compat-minicluster-profile-2/java/org/apache/impala/compat/HdfsShim.java@23
PS1, Line 23: diffences betwen
typo


http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/10487/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@67
PS1, Line 67: import org.apache.impala.compat.HdfsShim;
The imports should be sorted



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 23 May 2018 18:20:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

2018-05-23 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10487


Change subject: IMPALA-7063: Fix compilation for MiniProfile2 after Erasure 
Coding changes.
..

IMPALA-7063: Fix compilation for MiniProfile2 after Erasure Coding changes.

This commit shims out HDFS's FileStatus.isErasureCoded() to manage
working with multiple versions of Hadoop.

I tested compilation with both profiles.

Cherry-picks: not for 2.x.

Change-Id: I423087078f84b0806545322519f224d58815123d
---
A 
fe/src/compat-minicluster-profile-2/java/org/apache/impala/compat/HdfsShim.java
A 
fe/src/compat-minicluster-profile-3/java/org/apache/impala/compat/HdfsShim.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
3 files changed, 63 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I423087078f84b0806545322519f224d58815123d
Gerrit-Change-Number: 10487
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Attila Jeges (Code Review)
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10486 )

Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG@29
PS1, Line 29:
I think you should add "Cherry-picks: not for 2.x." line.


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h
File be/src/exprs/timezone_db.h:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55
PS1, Line 55: map
Maybe use unordered_map instead ? Probably wouldn't make much of a difference 
though.


http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc
File be/src/exprs/timezone_db.cc:

http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc@68
PS1, Line 68: {"AEST", "Australia/Sydney"},
: {"CDT", "America/Chicago"},
: {"CEST", "CET"},
: {"EDT", "EST5EDT"},
: {"ICT", "Asia/Ho_Chi_Minh"},
: {"KST", "Asia/Seoul"},
: {"MDT", "MST7MDT"},
: {"PHT", "Asia/Manila"},
: {"PDT", "America/Los_Angeles"}
Are these time-zone abbreviations accepted by Hive? If not, I think we should 
remove them.


http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv
File testdata/data/timezoneverification.csv:

http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv@716
PS1, Line 716: JST
Please make sure hat all the Java-supported time-zone abbreviations are tested 
here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Comment-Date: Wed, 23 May 2018 17:53:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7044: Prevent overflow when computing Parquet block size

2018-05-23 Thread Thomas Marshall (Code Review)
Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10483 )

Change subject: IMPALA-7044: Prevent overflow when computing Parquet block size
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-parquet-table-writer.cc
File be/src/exec/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-parquet-table-writer.cc@795
PS1, Line 795: table_desc_->num_cols() - table_desc_->num_clustering_cols()
num_cols


http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:

http://gerrit.cloudera.org:8080/#/c/10483/1/be/src/exec/hdfs-table-sink.cc@376
PS1, Line 376: return Status("HDFS block size must be smaller than 2GB.");
Any reasonable way to test this path?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e63420e5a093c0bbc789201771708865b16e138
Gerrit-Change-Number: 10483
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Wed, 23 May 2018 17:45:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7058: disable fuzz test for RC and Seq

2018-05-23 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10485 )

Change subject: IMPALA-7058: disable fuzz test for RC and Seq
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 23 May 2018 17:35:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7060: Restrict Impala to only support timezones that work in Hive

2018-05-23 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10486


Change subject: IMPALA-7060: Restrict Impala to only support timezones that 
work in Hive
..

IMPALA-7060: Restrict Impala to only support timezones that work in Hive

IANA timezone integration (IMPALA-3307) will drop the support for
many non-standard timezone aliases. As IMPALA-3307 is a very large
change, its release may be delayed - meanwhile, it would be better
to discourage Impala 3.x users from using timezone names that will
not be supported in the future. For this reason, this change drops
the support of non-standard aliases from the current boost based
implementation.

This is supposed to be a temporary solution till IMPALA-3307, so
I have put minimal effort into it. It should be much faster than
the previous solution for timezone aliases, while slightly slower
for canonical names.

Testing:
Timestamp aliases were largely untested, so only a few changes were
needed - these were copied from the review for IMPALA-3307,
gerrit.cloudera.org/#/c/9986/ It may be useful to add more tests,
e.g. to check that using dropped aliases return NULL and lead to
warning, but I think that these kind of changes should be done first
in the review of IMPALA-3307, and synced here afterwards.

Change-Id: I90859398081bae4976af31b09b3121c198b6adac
---
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timezone_db.cc
M be/src/exprs/timezone_db.h
M testdata/data/timezoneverification.csv
4 files changed, 46 insertions(+), 29 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac
Gerrit-Change-Number: 10486
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 


[Impala-ASF-CR] IMPALA-7058: disable fuzz test for RC and Seq

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10485 )

Change subject: IMPALA-7058: disable fuzz test for RC and Seq
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 23 May 2018 17:02:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7058: disable fuzz test for RC and Seq

2018-05-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10485


Change subject: IMPALA-7058: disable fuzz test for RC and Seq
..

IMPALA-7058: disable fuzz test for RC and Seq

There appear to still be some rare crashes. Let's disable the
test until we can sort those out

Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
---
M tests/query_test/test_scanners_fuzz.py
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I10eb184ab2f27ca9b2d286630ceb37b71affcc27
Gerrit-Change-Number: 10485
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7061: Rework HBase splitting and assignment

2018-05-23 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10447 )

Change subject: IMPALA-7061: Rework HBase splitting and assignment
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/bin/generate-schema-statements.py
File testdata/bin/generate-schema-statements.py:

http://gerrit.cloudera.org:8080/#/c/10447/6/testdata/bin/generate-schema-statements.py@487
PS6, Line 487: {SPLITS => ['1', '3', '5', '7', '9']}"
Thinking about how to avoid hard-coding this (or at least making it clear what 
is happening)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
Gerrit-Change-Number: 10447
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 23 May 2018 16:53:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-6131: Track time of last statistics update in metadata

2018-05-23 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10484 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Gerrit-Change-Number: 10484
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Comment-Date: Wed, 23 May 2018 16:49:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7061: Rework HBase splitting and assignment

2018-05-23 Thread Joe McDonnell (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7061: Rework HBase splitting and assignment
..

IMPALA-7061: Rework HBase splitting and assignment

Some frontend PlannerTests rely on HBase tables being
arranged in a deterministic way. Specifically, the
HBase tables need to be split with specific region
boundaries and those regions need to be assigned to
specific HBase region servers.

Currently, the tables are created without splits and
testdata/bin/split-hbase.sh runs Java code in
HBaseTestDataRegionAssignment to split and assign
the tables. This runs during dataload via
testdata/bin/create-load-data.sh and during tests
with bin/run-all-tests.sh. There are problems with
both parts of this process. The table splitting is
flaky. Since significant time can pass between the
assignments and the tests, rebalancing means the
assignments are not always stable.

This changes the process so that the HBase tables are
created with the splits already specified via the
HBase shell. The splits remain stable over time.
PlannerTestBase runs the assignment code in
HBaseTestDataRegionAssignment at the start of
the PlannerTests. This makes the assignments
deterministic. No other tests depends on the
exact assignments, so this does not regress anything.

Testing:
 - Local testing
 - Ran gerrit-verify-dryrun-external
 - Verified minicluster profile 2 compiles

Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
---
M bin/run-all-tests.sh
A 
fe/src/compat-minicluster-profile-2/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
A 
fe/src/compat-minicluster-profile-3/test/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssignment.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/bin/create-load-data.sh
M testdata/bin/generate-schema-statements.py
D testdata/bin/split-hbase.sh
M testdata/datasets/functional/functional_schema_template.sql
D 
testdata/src/compat-minicluster-profile-2/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssigment.java
D 
testdata/src/compat-minicluster-profile-3/java/org/apache/impala/datagenerator/HBaseTestDataRegionAssigment.java
10 files changed, 362 insertions(+), 728 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3d639128a856254a6ccb93d6750f531974b5f897
Gerrit-Change-Number: 10447
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

2018-05-23 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 15:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8523/15/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/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@783
PS15, Line 783:   boolean checkMissingDiskIds = 
FileSystemUtil.supportsStorageIds(partitionFs);
  :   boolean supportsBlocks = 
FileSystemUtil.supportsStorageIds(partitionFs);
these are equivalent. how about getting rid of the checkMissingDiskIds, and 
renaming supportsBlocks to fsSupportsBlocks?


http://gerrit.cloudera.org:8080/#/c/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@812
PS15, Line 812: result.first
was changing the polarity of this a bug fix?


http://gerrit.cloudera.org:8080/#/c/8523/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@914
PS15, Line 914: checkMissingDiskIds && !fileDesc.getIsEc()
this was confusing because when I read the name "checkMissingDiskIds" I 
thought: why doesn't the setting of that variable take into account EC?
How about renaming that parameter to: fsHasBlocks or fsHasDiskIds or something 
to indicate that's a filesystem level property?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 15
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 23 May 2018 15:37:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7048: Failed test: test write index many columns tables

2018-05-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10476 )

Change subject: IMPALA-7048: Failed test: test_write_index_many_columns_tables
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd3be70fb654a49dda44309a8914fe1f2b48a1af
Gerrit-Change-Number: 10476
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 23 May 2018 15:19:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests

2018-05-23 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10442 )

Change subject: IMPALA-6802 (part 4): Clean up authorization tests
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4
Gerrit-Change-Number: 10442
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 23 May 2018 14:30:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests

2018-05-23 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10442 )

Change subject: IMPALA-6802 (part 4): Clean up authorization tests
..


Patch Set 1:

(11 comments)

Taking this over since Adam is out.

http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@104
PS1, Line 104:   private static final String[] ALLTYPES_COLUMNS = (String []) 
ArrayUtils.addAll(
> nit: String[]
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@923
PS1, Line 923: .ok(onServer(TPrivilegeLevel.INSERT))
> We need to add REFRESH since VIEW_METADATA consists of SELECT, INSERT, and
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@927
PS1, Line 927: .error(accessError("functional"));
> Missing check .error(accessError("functional"), onDatabase...)
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@933
PS1, Line 933: TTableName tableName = new 
TTableName("functional","alltypes");
> nit: space after comma
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@935
PS1, Line 935: authorize("describe functional.alltypes")
> describe  uses ANY privilege, we need more test cases with all other
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@961
PS1, Line 961: String [] locationString = new String[]{"Location:"};
> nit: no space for array declaration
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@989
PS1, Line 989: tableName = new TTableName("functional","alltypes_view");
> nit: space after comma
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1013
PS1, Line 1013: tableName = new TTableName("functional","alltypes_view");
> nit: space after comma
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1018
PS1, Line 1018: viewStrings);
> join L1018 with L1017
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1040
PS1, Line 1040: authorize("describe 
functional.allcomplextypes.int_struct_col")
> describe  uses ANY privilege, we need more test cases with all other
Done


http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1183
PS1, Line 1183: String [] requiredStrings, 
String [] excludedStrings,
> nit: fix indentation
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4
Gerrit-Change-Number: 10442
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 23 May 2018 14:29:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-6131: Track time of last statistics update in metadata

2018-05-23 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10484 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 1:

The commit on master ( https://gerrit.cloudera.org/#/c/10116/ ) changed 
AuthorizationTest.java to fix a broken test. This test is not present in 2.x, 
which made the changes in that file unnecessary + conflicting.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibda49725d3e76456f2d1b3edd1bf117b0174e234
Gerrit-Change-Number: 10484
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Comment-Date: Wed, 23 May 2018 11:55:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5392: Added all stack frames to ThreadInfo summary.

2018-05-23 Thread Abhishek Sharma (Code Review)
Abhishek Sharma has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10145 )

Change subject: IMPALA-5392: Added all stack frames to ThreadInfo summary.
..


Patch Set 10:

Hi Jim,

Please ignore the last comment. I understand your concern. I checked the 
current implementation, the same issue holds there as well. HTML DOES NOT 
respect "newlines" and "tabs".

Another point, as per my understanding, this API MUST NOT worry about how it is 
rendered in HTML. The responsibility of rendering it properly must lie with a 
frontend component. That is, we SHOULD NOT add HTML line breaks in this API's 
return string.

Please do let me know how should I address your concern.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80ab4aad03e0c1f01fecad6b87779531244c28b7
Gerrit-Change-Number: 10145
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Sharma 
Gerrit-Reviewer: Abhishek Sharma 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Charles Agnello 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 23 May 2018 11:53:55 +
Gerrit-HasComments: No


  1   2   >