[Impala-ASF-CR] IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners

[Pranay Singh (Code Review)]

Hello anujphadke, Tim Armstrong,

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

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

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

Change subject: IMPALA-3833: Fix invalid data handling in Sequence and RCFile 
scanners
..

IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners

Enabled the fuzz test for Sequence and RCFiles and added new checks to return
when failure is encountered while reading an RC file or sequence file.

Testing:
  Ran the fuzz test on a private build without failures/crash.

Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
---
M be/src/exec/hdfs-rcfile-scanner.cc
M be/src/exec/hdfs-rcfile-scanner.h
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/exec/hdfs-sequence-scanner.h
M be/src/exec/read-write-util-test.cc
M be/src/exec/read-write-util.h
M be/src/exec/scanner-context.inline.h
M be/src/util/decompress.cc
M tests/query_test/test_scanners_fuzz.py
9 files changed, 218 insertions(+), 61 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 8936
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6394: Enable HDFS debug logging

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9082 )

Change subject: IMPALA-6394: Enable HDFS debug logging
..


Patch Set 1: Code-Review+1

Did you do a private run with these new settings? How does this change affect 
the log size? Does it affect test runtime?

I'm generally ok with this change.

As an alternative we could also set up a separate job that runs continuously 
and only does the data load + waiting portion with this change so we can 
hopefully catch the issue in action. That way we don't have to change this in 
master.

I'll +1 but would like a second pair of eyes to see whether there is a 
preference for this or the alternative approach.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide09d5abb141dbbb6bc1ee66b69144ac41f841d9
Gerrit-Change-Number: 9082
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Sat, 20 Jan 2018 06:05:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6427: fix QUERYOPTIONS in planner test output

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9081 )

Change subject: IMPALA-6427: fix QUERYOPTIONS in planner test output
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2971588f977f72b0f869370803b089d56303d91a
Gerrit-Change-Number: 9081
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Sat, 20 Jan 2018 05:14:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6427: fix QUERYOPTIONS in planner test output

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9081 )

Change subject: IMPALA-6427: fix QUERYOPTIONS in planner test output
..

IMPALA-6427: fix QUERYOPTIONS in planner test output

Testing:
Ran planner tests. Confirmed that the generated union.test
included the QUERYOPTIONS section and that test files without the
QUERYOPTIONS section did not have QUERYOPTIONS sections added.

Change-Id: I2971588f977f72b0f869370803b089d56303d91a
Reviewed-on: http://gerrit.cloudera.org:8080/9081
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
1 file changed, 7 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2971588f977f72b0f869370803b089d56303d91a
Gerrit-Change-Number: 9081
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 5: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr-test.cc@279
PS5, Line 279:   //RunMultipleServicesTestTemplate(
 :   //static_cast(this), 
_mgr_, krpc_address_);
delete


http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/5/be/src/rpc/rpc-mgr.cc@100
PS5, Line 100: bld.set_rpc_encryption("required");
Makes me wonder if we should have a test case in which remote nodes somehow 
doesn't support encryption ? That said, it's not a supported configuration.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 20 Jan 2018 03:26:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8950 )

Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc@703
PS3, Line 703: FLAGS_datastream_service_num_deserialization_threads = 1;
 : FLAGS_datastream_service_deserialization_queue_size = 1;
> So this class is not reusable for other tests. It's hard to make a class th
This alternative is not ideal either. I guess this is not too important for now 
so I am okay with the current approach given we document on why it's built this 
way.


http://gerrit.cloudera.org:8080/#/c/8950/5/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/8950/5/be/src/runtime/data-stream-test.cc@658
PS5, Line 658:  info.status = sender.Send(
Given there is nothing special about this class, may be this should be called 
DataStreamTestThriftOnly ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
Gerrit-Change-Number: 8950
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 20 Jan 2018 03:12:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6092: run flaky test serially (temporary).

[Vuk Ercegovac (Code Review)]

Hello Dimitris Tsirogiannis, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6092: run flaky test serially (temporary).
..

IMPALA-6092: run flaky test serially (temporary).

The reason for the flake is described in IMPALA-6215.
This patch runs the test serially to help reduce the
recent increase of flakes on gvo.

Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
---
M tests/query_test/test_udfs.py
1 file changed, 2 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
Gerrit-Change-Number: 9080
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] Bumping version to 3.0.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9044 )

Change subject: Bumping version to 3.0.
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39f9648cb9b40b67b1029fa8c4132cd04c1d0c6
Gerrit-Change-Number: 9044
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 20 Jan 2018 02:10:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6420: Fix TestCharFormats for local filesystem tests

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9074 )

Change subject: IMPALA-6420: Fix TestCharFormats for local filesystem tests
..

IMPALA-6420: Fix TestCharFormats for local filesystem tests

Tl;dr - running tests was covering up a bug in the tests,
or alternatively, not running tests exposes a bug.

Local tests were failing because we failed to prepend
the local filesystem prefix.  This was covered up because
the snapshot creation job runs tests, which actually creates
these tables in the metastore.  When running local filesystem
tests with a snapshot, the table creation never runs because
the snapshot import converts hdfs:// locations into local
file locations.  The problematic CREATE TABLE x IF NOT EXISTS
then never get run, hiding the problem.

Testing: With a local filesystem install, in Impala shell:
drop table functional_parquet.chars_formats

and then:
 tests/run-tests.py  query_test/test_chars.py -k char_format

Change-Id: I916b5566a3c5a83185e502b814ad43b5af14e87f
Reviewed-on: http://gerrit.cloudera.org:8080/9074
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M tests/query_test/test_chars.py
1 file changed, 9 insertions(+), 8 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I916b5566a3c5a83185e502b814ad43b5af14e87f
Gerrit-Change-Number: 9074
Gerrit-PatchSet: 3
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6420: Fix TestCharFormats for local filesystem tests

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9074 )

Change subject: IMPALA-6420: Fix TestCharFormats for local filesystem tests
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I916b5566a3c5a83185e502b814ad43b5af14e87f
Gerrit-Change-Number: 9074
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 20 Jan 2018 01:52:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9060 )

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 1:

(1 comment)

Did we intentionally not update the use of SSLeay() in 
be/src/thirdparty/squeasel/squeasel.c ?

http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/rpc/thrift-server.cc@94
PS1, Line 94:   bool is_openssl_1_0_0_or_lower = (max_supported_tls_version < 
TLS1_1_VERSION);
:   if (is_openssl_1_0_0_or_lower) return (protocol == 
TLSv1_0_plus);
> We make that assumption since those can be the only values. We always get t
Do you think it makes sense to have a DCHECK to document this assumption ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 20 Jan 2018 01:10:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6338: Fix flaky test profile fragment instances

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8997 )

Change subject: IMPALA-6338: Fix flaky test_profile_fragment_instances
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8997/2/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

http://gerrit.cloudera.org:8080/#/c/8997/2/be/src/runtime/coordinator-backend-state.h@206
PS2, Line 206: either because the client called Cancel() or because the 
coordinator
 :   /// initiated cancel because all the results were already 
returned
It could also be because another backend hit an error.

Do we know of any other ways cancellation can be initiated? See more in my 
comment below.


http://gerrit.cloudera.org:8080/#/c/8997/2/be/src/runtime/coordinator-backend-state.h@243
PS2, Line 243: other than cancel
We're making a base assumption that a CANCELLED status is only obtained as a 
result of the Coordinator setting and propagating it (for any reason).

But are we sure that's true (It would be ideal if that were true)? i.e. are we 
sure that there's no fragment instance cancelling itself (and hence the query) 
for some other reason?

Because if that was the case, then with this patch, a fragment instance trying 
to cancel the query wouldn't cause the query to be cancelled. Since the 
coordinator is assuming that if it sees a CANCELLED status from a fragment 
instance, that's due to the coordinator setting it.


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

http://gerrit.cloudera.org:8080/#/c/8997/2/be/src/runtime/coordinator.cc@926
PS2, Line 926: // If the query was cancelled by the user, don't process the 
update.
 :   if (query_status_.IsCancelled()) return Status::OK();
Shouldn't we tell the backend to cancel itself if the query was cancelled by 
the user? Or are we relying on the Cancel() RPCs doing that?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a1e3c62952003f37f88fe2b662bb11889ed
Gerrit-Change-Number: 8997
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:45:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6427: fix QUERYOPTIONS in planner test output

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9081 )

Change subject: IMPALA-6427: fix QUERYOPTIONS in planner test output
..


Patch Set 1: Code-Review+2

thx for fixing


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2971588f977f72b0f869370803b089d56303d91a
Gerrit-Change-Number: 9081
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:44:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6092: turn off flaky test temporarily.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9080 )

Change subject: IMPALA-6092: turn off flaky test temporarily.
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
Gerrit-Change-Number: 9080
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:40:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6092: turn off flaky test temporarily.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9080 )

Change subject: IMPALA-6092: turn off flaky test temporarily.
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
Gerrit-Change-Number: 9080
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:13:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6070: Expose using Docker to run tests faster.

[Philip Zeyliger (Code Review)]

Philip Zeyliger has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9085


Change subject: IMPALA-6070: Expose using Docker to run tests faster.
..

IMPALA-6070: Expose using Docker to run tests faster.

Allows running the tests that make up the "core" suite in about 2 hours.
By comparison, 
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/buildTimeTrend
tends to run in about 3.5 hours.

This commit:
* Adds "echo" statements in a few places, to facilitate timing.
* Adds --skip-parallel/--skip-serial flags to run-tests.py,
  and exposes them in run-all-tests.sh.
* Adds "test-with-docker.py", which runs a full build, data load,
  and executes tests inside of Docker containers, generating
  a timeline at the end. In short, one container is used
  to do the build and data load, and then this container is
  re-used to run various tests in parallel. All logs are
  left on the host system.

Besides the obvious win of getting test results more quickly, this
commit serves as an example of how to get various bits of Impala
development working inside of Docker containers. For example, Kudu
relies on atomic rename of directories, which isn't available in most
Docker filesystems, and entrypoint.sh works around it.

In addition, the timeline generated by the build suggests where further
optimizations can be made. Most obviously, dataload eats up a precious
~30-50 minutes, on a largely idle machine.

This work is significantly CPU and memory hungry. It was developed on a
32-core, 120GB RAM Google Compute Engine machine, and I know that 75GB
of those are currently used. By and large, EC2 and GCE price machines
linearly, so, if CPU usage can be kept up, it's not wasteful to run on
bigger machines.

I've run into some fragility with the tests. Specifically,
thrift-server-test fails consistently, as it does on my development
Ubuntu 16.04 machine as well.

Change-Id: I82052ef31979564968effef13a3c6af0d5c62767
---
M bin/bootstrap_system.sh
M bin/run-all-tests.sh
M buildall.sh
A docker/annotate.pl
A docker/annotate.py
A docker/entrypoint.sh
A docker/test-with-docker.py
M testdata/bin/run-all.sh
M tests/run-tests.py
9 files changed, 1,013 insertions(+), 16 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I82052ef31979564968effef13a3c6af0d5c62767
Gerrit-Change-Number: 9085
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Philip Zeyliger 

[native-toolchain-CR] Bump Kudu version to c6beta-impala-toolchain-tag1

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9086


Change subject: Bump Kudu version to c6beta-impala-toolchain-tag1
..

Bump Kudu version to c6beta-impala-toolchain-tag1

Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
---
M buildall.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
Gerrit-Change-Number: 9086
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-6092: turn off flaky test temporarily.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9080 )

Change subject: IMPALA-6092: turn off flaky test temporarily.
..


Patch Set 1:

Let's file a follow-up JIRA to re-enable this as a release blocker, otherwise 
we'll be missing a lot of coverage.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
Gerrit-Change-Number: 9080
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jan 2018 23:56:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6075: Add Impala daemon metric for catalog version.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8949 )

Change subject: IMPALA-6075: Add Impala daemon metric for catalog version.
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Gerrit-Change-Number: 8949
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Fri, 19 Jan 2018 23:50:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6092: turn off flaky test temporarily.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9080 )

Change subject: IMPALA-6092: turn off flaky test temporarily.
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a4165f0e82f270df2c8a3af087a9a6ec63fd086
Gerrit-Change-Number: 9080
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Fri, 19 Jan 2018 23:47:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..

IMPALA-6422: Use ldexp() instead of powf() in HLL.

Using ldexp() to compute a floating point power of two is
over 10x faster than powf().

This change is particularly helpful for speeding up
COMPUTE STATS TABLESAMPLE which has many calls to
HllFinalEstimate() where floating point power of two
computations are relevant.

Testing:
- core/hdfs run passed

Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Reviewed-on: http://gerrit.cloudera.org:8080/9078
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/exec/incr-stats-util.cc
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
3 files changed, 9 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 23:22:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6105 unix timestamp returns a number of seconds.

[Alex Rodoni (Code Review)]

Alex Rodoni has abandoned this change. ( http://gerrit.cloudera.org:8080/9083 )

Change subject: IMPALA-6105 unix_timestamp returns a number of seconds.
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ieadde4cdf7bf3618ed5b8e489da56597a615ec9a
Gerrit-Change-Number: 9083
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 

[Impala-ASF-CR] IMPALA-6105 unix timestamp returns a number of seconds.

[Alex Rodoni (Code Review)]

Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9083


Change subject: IMPALA-6105 unix_timestamp returns a number of seconds.
..

IMPALA-6105 unix_timestamp returns a number of seconds.

Change-Id: Ieadde4cdf7bf3618ed5b8e489da56597a615ec9a
---
M docs/topics/impala_datetime_functions.xml
1 file changed, 45 insertions(+), 38 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieadde4cdf7bf3618ed5b8e489da56597a615ec9a
Gerrit-Change-Number: 9083
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 3: Code-Review+2

(3 comments)

Thanks for the reviews. I addressed all comments in PS3. Carrying Sailesh's +2.

http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG@10
PS2, Line 10: there was a bug in the retry logic
> Could you also briefly add what this bug was in the commit message?
Done


http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG@15
PS2, Line 15: oves the sleep()
> typo
Done


http://gerrit.cloudera.org:8080/#/c/9079/2/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/9079/2/tests/query_test/test_observability.py@151
PS2, Line 151: introduce an error below
> I don't think that it's important to state this line. But if you feel like
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 22:57:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 22:57:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Hello Sailesh Mukil, Zoram Thanga,

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

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

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..

IMPALA-6399: Fix timeout logic in test_query_profile_thrift_timestamps

In the previous fix for IMPALA-6399 we increased the number of retries.
However, there was a bug in the retry logic: If the profile was missing,
we would call 'continue' without sleeping, thus eating through the
maximum number of retries in a short period of time.

This change now switches to a time-based wait-loop instead of a set
number of retries. It also moves the sleep() to the beginning of the
loop to make it less likely to forget  it when changing the code in the
future. Calling sleep() before trying to fetch the profile for the first
time also reduces the likelihood of hitting a warning when the profile
is not yet available.

We also change the timeout back to 60 seconds which should be enough for
the profile to show up.

Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
---
M tests/query_test/test_observability.py
1 file changed, 16 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 2: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG@10
PS2, Line 10: there was a bug in the retry logic
Could you also briefly add what this bug was in the commit message?


http://gerrit.cloudera.org:8080/#/c/9079/2//COMMIT_MSG@15
PS2, Line 15: to forget the it
typo


http://gerrit.cloudera.org:8080/#/c/9079/2/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/9079/2/tests/query_test/test_observability.py@151
PS2, Line 151: introduce an error below
I don't think that it's important to state this line. But if you feel like it's 
better to keep it, I have a nit:
"...introduce an error below in future changes..."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 22:38:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6394: Enable HDFS debug logging

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9082


Change subject: IMPALA-6394: Enable HDFS debug logging
..

IMPALA-6394: Enable HDFS debug logging

Recently some builds fails at waiting for hdfs replication in data
loading. This patch set the HDFS logging level to DEBUG to help diagnose
this problem.

Change-Id: Ide09d5abb141dbbb6bc1ee66b69144ac41f841d9
---
M testdata/cluster/node_templates/common/etc/hadoop/conf/log4j.properties.tmpl
M testdata/cluster/node_templates/common/etc/init.d/hdfs-common
2 files changed, 2 insertions(+), 2 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 4:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@78
PS4, Line 78: string IMPALA_HOME(getenv("IMPALA_HOME"));
> const static
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@90
PS4, Line 90: TLSv1.0 compatible ciphers
> Are there ways to detect the TLS version supported on the platform and choo
There actually is a way now, but the patch that introduces some basic utils for 
it is still in review here:
https://gerrit.cloudera.org/#/c/9060/

I can add tests for TLSv1.2 as a follow on patch.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@268
PS4, Line 268: RunTlsTestTemplate
> What do you think about factoring line 331 to line 371 below into a single
Done. I removed RunTlsTestTemplate() and made every test use the new function 
RunMultipleServicesTestTemplate()


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@309
PS4, Line 309:
> nit: blank line
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@331
PS4, Line 331:   // Test that a service can be started, and will respond to 
requests.
 :   unique_ptr ping_impl(
 :   new PingServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(ping_impl)));
 :
 :   // Test that a second service, that verifies the RPC payload 
is not corrupted,
 :   // can be started.
 :   unique_ptr scan_mem_impl(
 :   new ScanMemServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(scan_mem_impl)));
 :
 :   FLAGS_num_acceptor_threads = 2;
 :   FLAGS_num_reactor_threads = 10;
 :   ASSERT_OK(tls_rpc_mgr.StartServices(tls_krpc_address));
 :
 :   unique_ptr ping_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_proxy));
 :
 :   unique_ptr scan_mem_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_mem_proxy));
 :
 :   RpcController controller;
 :   srand(0);
 :   // Randomly invoke either services to make sure a RpcMgr can 
host multiple
 :   // services at the same time.
 :   for (int i = 0; i < 100; ++i) {
 : controller.Reset();
 : if (random() % 2 == 0) {
 :   PingRequestPB request;
 :   PingResponsePB response;
 :   kudu::Status status = ping_proxy->Ping(request, , 
);
 :   ASSERT_TRUE(status.ok());
 :   ASSERT_EQ(response.int_response(), 42);
 : } else {
 :   ScanMemRequestPB request;
 :   ScanMemResponsePB response;
 :   SetupScanMemRequest(, );
 :   kudu::Status status = scan_mem_proxy->ScanMem(request, 
, );
 :   ASSERT_TRUE(status.ok());
 : }
 :   }
> Please see comments above on how this may be factored into a function to be
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@378
PS4, Line 378:   // Misconfigure TLS with bad CA certificate.
> nit: may as well move it to before line 377 as block comment to describe wh
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@392
PS4, Line 392:
 :   // Misconfigure TLS with a bad password command for the 
passowrd protected private key.
> nit: may as well move it to before line 391 as block comment for this funct
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@412
PS4, Line 412:   // Configure TLS with a password protected private key and the 
correct password for it.
> nit: same here.
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@430
PS4, Line 430:   // Misconfigure TLS with a bad cipher.
> nit: same here
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@446
PS4, Line 446:   // Enable TLS with a TLSv1 compatible cipher list.
> nit: same here.
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h@100
PS4, Line 100: {
 :   }
> nit: one line { }
Done


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Dan Hecht,

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

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

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

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..

IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

KRPC has some flags that turn on TLS. This patch sets those to enable
TLS communication.

Tests are added to rpc-mgr-test.

Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/authentication-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
15 files changed, 329 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6420: Fix TestCharFormats for local filesystem tests

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9074 )

Change subject: IMPALA-6420: Fix TestCharFormats for local filesystem tests
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I916b5566a3c5a83185e502b814ad43b5af14e87f
Gerrit-Change-Number: 9074
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 22:07:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6420: Fix TestCharFormats for local filesystem tests

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9074 )

Change subject: IMPALA-6420: Fix TestCharFormats for local filesystem tests
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9074/2/tests/query_test/test_chars.py
File tests/query_test/test_chars.py:

http://gerrit.cloudera.org:8080/#/c/9074/2/tests/query_test/test_chars.py@93
PS2, Line 93:   def __create_char_tables(self):
> I think since this relies on an external file populated in an earlier data
We have a different flow in a couple of other places like test_scanners.py 
TestParquet, but I'm ok with this simpler fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I916b5566a3c5a83185e502b814ad43b5af14e87f
Gerrit-Change-Number: 9074
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 21:56:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6427: fix QUERYOPTIONS in planner test output

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9081


Change subject: IMPALA-6427: fix QUERYOPTIONS in planner test output
..

IMPALA-6427: fix QUERYOPTIONS in planner test output

Testing:
Ran planner tests. Confirmed that the generated union.test
included the QUERYOPTIONS section and that test files without the
QUERYOPTIONS section did not have QUERYOPTIONS sections added.

Change-Id: I2971588f977f72b0f869370803b089d56303d91a
---
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
1 file changed, 7 insertions(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from TABLESAMPLE clause

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9066 )

Change subject: IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from 
TABLESAMPLE clause
..

IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from TABLESAMPLE clause

Overlooked the new keyword when the clause was
originally introduced.

Change-Id: Ie8e6713fb97ced279f0aedfe8f42c09a7e6edae9
Reviewed-on: http://gerrit.cloudera.org:8080/9066
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_reserved_words.xml
1 file changed, 1 insertion(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie8e6713fb97ced279f0aedfe8f42c09a7e6edae9
Gerrit-Change-Number: 9066
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from TABLESAMPLE clause

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9066 )

Change subject: IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from 
TABLESAMPLE clause
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8e6713fb97ced279f0aedfe8f42c09a7e6edae9
Gerrit-Change-Number: 9066
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 21:26:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from TABLESAMPLE clause

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9066 )

Change subject: IMPALA-5310: [DOCS] Reserve 'repeatable' keyword from 
TABLESAMPLE clause
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie8e6713fb97ced279f0aedfe8f42c09a7e6edae9
Gerrit-Change-Number: 9066
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 19 Jan 2018 21:17:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6418: Find a reliable way to detect supported TLS versions

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9060 )

Change subject: IMPALA-6418: Find a reliable way to detect supported TLS 
versions
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/rpc/thrift-server.cc@94
PS1, Line 94:   bool is_openssl_1_0_0_or_lower = (max_supported_tls_version < 
TLS1_1_VERSION);
:   if (is_openssl_1_0_0_or_lower) return (protocol == 
TLSv1_0_plus);
if (max_supported_tls_version < TLS_1_1_VERSION) {
return protocol == TLSv1_0_plus;
}

Also, not necessarily your change, but should this actually be:
 protocol == TLSv1_0_plus || protocol == TLSv1_0 ?

This function seems to make the assumption that protocol will only be of values 
TLSv1_2_plus || TLS_v1_1_plus || TLSv1_0_plus ? Not sure if this is a good 
assumption to make in general ?


http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/9060/1/be/src/util/openssl-util.h@47
PS1, Line 47: int MaxSupportedTlsVersion();
Please add a quick comment about this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:53:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..

IMPALA-4993: extend dictionary filtering to collections

Currently, top-level scalar columns in parquet files can
be used at runtime to prune row-groups by evaluating certain
conjuncts over the column's dictionary (if available).

This change extends such pruning to scalar values that are
stored in collection type columns. Currently, dictionary
pruning works by finding eligible conjuncts for top-level
slots. Since only top-level slots are supported, the slots
are implicitly part of the scan node's tuple descriptor.
With this change, we track eligible conjuncts by slot as well
as the tuple that contains the slot (either top-level or
nested collection). Since collection conjuncts are already
managed by a map that associates tuple descriptors to a list
of their conjuncts, this extension follows the existing
representation.

The frontend builds the mapping of SlotId to conjuncts that
are dictionary filterable. This mapping now includes SlotId's
that reference nested tuples. The backend is adjusted to
use the same representation. In addition, collection
readers are decomposed into scalar filterable columns and
other, non-dictionary filterable readers. When filtering
a row group using a conjunct associated to a (possibly)
nested collection type, an additional tuple buffer is
allocated per tuple descriptor.

Testing:
- e2e test extended to illustrate row-groups that are pruned
  by nested collection dictionary filters.

Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Reviewed-on: http://gerrit.cloudera.org:8080/8775
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet-column-readers.cc
M be/src/runtime/collection-value-builder.h
M be/src/runtime/scoped-buffer.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A testdata/CustomerMultiBlock/README
A testdata/CustomerMultiBlock/customer_multiblock.parquet
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-filtering.test
20 files changed, 708 insertions(+), 248 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 22
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9077 )

Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..


Patch Set 1:

Thanks for providing an updated patch. Can you briefly describe how you tested 
this?


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:37:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..


Patch Set 21: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:37:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Zoram Thanga (Code Review)]

Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:24:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 1:

(1 comment)

Thanks for the review, please see PS2.

http://gerrit.cloudera.org:8080/#/c/9079/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/9079/1/tests/query_test/test_observability.py@164
PS1, Line 164: elapsed = time.time() - (end - MAX_WAIT)
> Can you add start = time.time() at L146, and use that to calculate the elap
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:22:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Hello Sailesh Mukil, Zoram Thanga,

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

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

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

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..

IMPALA-6399: Fix timeout logic in test_query_profile_thrift_timestamps

In the previous fix for IMPALA-6399 we increased the number of retries.
However, there was a bug in the retry logic that would prevent it from
sleeping when the profile was not available yet.

This change now switches to a time-based wait-loop instead of a set
number of retries. It also moves the sleep() to the beginning of the
loop to make it less likely to forget the it when changing the code in
the future. Calling sleep() before trying to fetch the profile for the
first time also reduces the likelihood of hitting a warning when the
profile is not yet available.

We also change the timeout back to 60 seconds which should be enough for
the profile to show up.

Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
---
M tests/query_test/test_observability.py
1 file changed, 16 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_mem_usage_scaling.py
M tests/query_test/test_tpcds_queries.py
23 files changed, 235 insertions(+), 139 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 6
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc@7688
PS5, Line 7688:   ColumnType::CreateDecimalType(15,2));
> Yes, same with below.  We should clean this up and make an RAII class WithE
Done


http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test
File testdata/workloads/functional-query/queries/QueryTest/uda.test:

http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test@85
PS5, Line 85: select agg_decimal_intermediate(cast(c3 as decimal(2,1)), 2), 
count(*)
> going on blind faith here
The test logic did not change as a result of this patch. We just change the 
table and column here to avoid the decimal cast error. Everything else is the 
same in this test.


http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py@443
PS5, Line 443: assert "Invalid base64 string; input length is 3, which is 
not a multiple of 4" in log
> ok, we're just testing get_log here
Correct


http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:

http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py@126
PS5, Line 126:   val, precision, 
vector.get_value('cast_from')).format(val, precision, scale)
> Unnecessary change.  Yes it is nicer, but it could cause more merge conflic
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:12:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:51:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Zoram Thanga (Code Review)]

Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 1:

(1 comment)

Thanks for fixing this.

http://gerrit.cloudera.org:8080/#/c/9079/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/9079/1/tests/query_test/test_observability.py@164
PS1, Line 164: elapsed = time.time() - (end - MAX_WAIT)
Can you add start = time.time() at L146, and use that to calculate the elapsed 
time instead? It would be more obvious that way.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:50:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Hello anujphadke, Tim Armstrong,

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

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

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

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..

IMPALA-6422: Use ldexp() instead of powf() in HLL.

Using ldexp() to compute a floating point power of two is
over 10x faster than powf().

This change is particularly helpful for speeding up
COMPUTE STATS TABLESAMPLE which has many calls to
HllFinalEstimate() where floating point power of two
computations are relevant.

Testing:
- core/hdfs run passed

Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
---
M be/src/exec/incr-stats-util.cc
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
3 files changed, 9 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:44:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] Bumping version to 3.0.

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9044 )

Change subject: Bumping version to 3.0.
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39f9648cb9b40b67b1029fa8c4132cd04c1d0c6
Gerrit-Change-Number: 9044
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:43:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9079 )

Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..


Patch Set 1:

Zoram, as the original author of the code, could you please have a look?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:31:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6399: Fix timeout logic in test query profile thrift timestamps

[Lars Volker (Code Review)]

Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9079


Change subject: IMPALA-6399: Fix timeout logic in 
test_query_profile_thrift_timestamps
..

IMPALA-6399: Fix timeout logic in test_query_profile_thrift_timestamps

In the previous fix for IMPALA-6399 we increased the number of retries.
However, there was a bug in the retry logic that would prevent it from
sleeping when the profile was not available yet.

This change now switches to a time-based wait-loop instead of a set
number of retries. It also moves the sleep() to the beginning of the
loop to make it less likely to forget the it when changing the code in
the future. Calling sleep() before trying to fetch the profile for the
first time also reduces the likelihood of hitting a warning when the
profile is not yet available.

We also change the timeout back to 60 seconds which should be enough for
the profile to show up.

Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
---
M tests/query_test/test_observability.py
1 file changed, 15 insertions(+), 14 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f0f3db75c52b8b2081dd15ed7e7d2a1d6b22389
Gerrit-Change-Number: 9079
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 

[Impala-ASF-CR] Bumping version to 3.0.

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9044 )

Change subject: Bumping version to 3.0.
..


Patch Set 3:

Are folks comfortable +2'ing this?

Once this goes in, I'll start maintaining the 2.x branch manually, 
cherrypicking most things (except this change). There's a tool to facilitate 
automating that in review, and I'll make it even a little bit more push-button 
after I do it manually for a few days.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39f9648cb9b40b67b1029fa8c4132cd04c1d0c6
Gerrit-Change-Number: 9044
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 19 Jan 2018 19:26:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Hello anujphadke, Tim Armstrong,

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

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

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

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..

IMPALA-6422: Use ldexp() instead of powf() in HLL.

Using ldexp() to compute a floating point power of two is
over 10x faster than powf().

This change is particularly helpful for speeding up
COMPUTE STATS TABLESAMPLE which has many calls to
HllFinalEstimate() where floating poing power of two
computations are relevant.

Testing:
- core/hdfs run passed

Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
---
M be/src/exec/incr-stats-util.cc
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/aggregate-functions.h
3 files changed, 9 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc@1489
PS1, Line 1489:   // TODO: Consider improving this loop (e.g. replacing 'if' 
with arithmetic op).
> Remove TODO while we're here? Seems more misleading than helpful since it's
Done


http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc@1512
PS1, Line 1512:   uint64_t estimate = HllFinalEstimate(src.ptr, src.len);
> I believe src.len is always HLL_LEN here. Might be worth passing the consta
I just removed the num_buckets arg from HllFinalEstimate() due to the 
DCHECK_EQ(num_buckets, HLL_LEN); in L1474.

Seems more honest to not pass num_buckets since we can only deal with HLL_LEN.

Please take another quick look.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 18:41:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc@7688
PS5, Line 7688:   ColumnType::CreateDecimalType(15,2));
> my understanding is we need to executor_->PopExecOption(); at the end of th
Yes, same with below.  We should clean this up and make an RAII class 
WithExecOption so this is automatic when leaving scope.  (Not in this diff)


http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test
File testdata/workloads/functional-query/queries/QueryTest/uda.test:

http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test@85
PS5, Line 85: select agg_decimal_intermediate(cast(c3 as decimal(2,1)), 2), 
count(*)
going on blind faith here


http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py@443
PS5, Line 443: assert "Invalid base64 string; input length is 3, which is 
not a multiple of 4" in log
ok, we're just testing get_log here


http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:

http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py@126
PS5, Line 126:   val, precision, 
vector.get_value('cast_from')).format(val, precision, scale)
Unnecessary change.  Yes it is nicer, but it could cause more merge conflicts 
going forward.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 18:17:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5440 Add planner tests with extreme statistics values

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9065 )

Change subject: IMPALA-5440 Add planner tests with extreme statistics values
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9065/2/testdata/workloads/functional-planner/queries/PlannerTest/cardinality.test
File testdata/workloads/functional-planner/queries/PlannerTest/cardinality.test:

http://gerrit.cloudera.org:8080/#/c/9065/2/testdata/workloads/functional-planner/queries/PlannerTest/cardinality.test@5
PS2, Line 5: |  Per-Host Resources: mem-estimate=0B mem-reservation=0B
I generally don't think this is the right way to approach this kind of testing.

Adding more .test files especially at higher explain levels makes future 
changes harder.

I think it makes most sense to write code to walk the plan and check that the 
cardinality is within legal bounds. The purpose of this testing is mostly to 
guard against regressions where query panning fails or produces nonsensical 
cardinalities (see IMPALA-5282 for the original motivation of adding these 
tests).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86dec47cf1438882cafaec53e97864ccfcdff3cb
Gerrit-Change-Number: 9065
Gerrit-PatchSet: 2
Gerrit-Owner: xyutin...@cloudera.com
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: xyutin...@cloudera.com
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:59:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Hello anujphadke,

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

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

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

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..

IMPALA-6422: Use ldexp() instead of powf() in HLL.

Using ldexp() to compute a floating point power of two is
over 10x faster than powf().

This change is particularly helpful for speeding up
COMPUTE STATS TABLESAMPLE which has many calls to
HllFinalEstimate() where floating point power of two
computations are relevant.

Testing:
- core/hdfs run passed

Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
---
M be/src/exprs/aggregate-functions-ir.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9078/1//COMMIT_MSG@14
PS1, Line 14: HllFinalEstimate() where floating poing power of two
> typo
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:54:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[anujphadke (Code Review)]

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 1: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9078/1//COMMIT_MSG@14
PS1, Line 14: HllFinalEstimate() where floating poing power of two
typo



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:49:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

[Alex Behm (Code Review)]

Alex Behm has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9078


Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..

IMPALA-6422: Use ldexp() instead of powf() in HLL.

Using ldexp() to compute a floating point power of two is
over 10x faster than powf().

This change is particularly helpful for speeding up
COMPUTE STATS TABLESAMPLE which has many calls to
HllFinalEstimate() where floating poing power of two
computations are relevant.

Testing:
- core/hdfs run passed

Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
---
M be/src/exprs/aggregate-functions-ir.cc
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc@7688
PS5, Line 7688:   ColumnType::CreateDecimalType(15,2));
my understanding is we need to executor_->PopExecOption(); at the end of these 
tests



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:29:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3998: Remove refresh after connect option from shell

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9072 )

Change subject: IMPALA-3998: Remove refresh_after_connect option from shell
..

IMPALA-3998: Remove refresh_after_connect option from shell

This removes the deprecated option in time for 3.0.

Testing:
Ran core tests. Manually ran the shell with the argument to confirm that
it reported "no such option".

Cherry-picks: not for 2.x.

Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
---
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
4 files changed, 0 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8f430bad0578e150d5e80066b9e7572041af4a15
Gerrit-Change-Number: 9072
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:03:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..


Patch Set 21: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:03:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded a new patch set (#21) to the change originally 
created by Vuk Ercegovac. ( http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..

IMPALA-4993: extend dictionary filtering to collections

Currently, top-level scalar columns in parquet files can
be used at runtime to prune row-groups by evaluating certain
conjuncts over the column's dictionary (if available).

This change extends such pruning to scalar values that are
stored in collection type columns. Currently, dictionary
pruning works by finding eligible conjuncts for top-level
slots. Since only top-level slots are supported, the slots
are implicitly part of the scan node's tuple descriptor.
With this change, we track eligible conjuncts by slot as well
as the tuple that contains the slot (either top-level or
nested collection). Since collection conjuncts are already
managed by a map that associates tuple descriptors to a list
of their conjuncts, this extension follows the existing
representation.

The frontend builds the mapping of SlotId to conjuncts that
are dictionary filterable. This mapping now includes SlotId's
that reference nested tuples. The backend is adjusted to
use the same representation. In addition, collection
readers are decomposed into scalar filterable columns and
other, non-dictionary filterable readers. When filtering
a row group using a conjunct associated to a (possibly)
nested collection type, an additional tuple buffer is
allocated per tuple descriptor.

Testing:
- e2e test extended to illustrate row-groups that are pruned
  by nested collection dictionary filters.

Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet-column-readers.cc
M be/src/runtime/collection-value-builder.h
M be/src/runtime/scoped-buffer.h
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
A testdata/CustomerMultiBlock/README
A testdata/CustomerMultiBlock/customer_multiblock.parquet
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-filtering.test
20 files changed, 708 insertions(+), 248 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/8775/21
--
To view, visit http://gerrit.cloudera.org:8080/8775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 21
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9032 )

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..


Patch Set 4:

(12 comments)

Thanks for working on this, the change seems pretty cool IMO.
Had some comments mostly about style.

http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/cpu-info.h
File be/src/util/cpu-info.h:

http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/cpu-info.h@37
PS4, Line 37:  = (1 << 1);
:   static const int64_t SSE4_1  = (1 << 2);
:   static const int64_t SSE4_2  = (1 << 3);
:   static const int64_t POPCNT  = (1 << 4);
:   static const int64_t AVX = (1 << 5);
:   static const int64_t AVX2= (1 << 6);
:   static const int64_t PCLMULQDQ =
please align the assignment operators


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc
File be/src/util/openssl-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc@47
PS4, Line 47: fill
nit: Fill, capital F


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc@49
PS4, Line 49: DCHECK_EQ(true, len >= 0);
DCHECK_GE(len, 0);


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc@118
PS4, Line 118: /// Test that encryption works with buffer lengths that don't 
fit in a 32-bit integer.
 : TEST_F(OpenSSLUtilTest, EncryptInPlaceHugeBuffer) {
 :   const int64_t buffer_size = 3 * 1024L * 1024L * 1024L;
 :   vector original(buffer_size);
 :   vector scratch(buffer_size); // Scratch buffer for 
in-place encryption.
 :   GenerateRandomData(original.data(), buffer_size);
 :
 :   // Check all the modes
 :   AES_CIPHER_MODE modes[] = {AES_256_GCM, AES_256_CTR, 
AES_256_CFB};
 :   for (auto m : modes) {
 : memcpy(scratch.data(), original.data(), buffer_size);
 : EncryptionKey key;
 : key.InitializeRandom();
 : key.SetCipherMode(m);
 : ASSERT_OK(key.Encrypt(scratch.data(), buffer_size, 
scratch.data()));
 : // Check that encryption did something
 : ASSERT_NE(0, memcmp(original.data(), scratch.data(), 
buffer_size));
 : ASSERT_OK(key.Decrypt(scratch.data(), buffer_size, 
scratch.data()));
 : // Check that we get the original data back.
 : ASSERT_EQ(0, memcmp(original.data(), scratch.data(), 
buffer_size));
 :   }
 : }
 :
 : /// Test that encryption works with arbitrary-length buffer
 : TEST_F(OpenSSLUtilTest, EncryptArbitraryLength) {
 :   std::uniform_int_distribution dis(0, 1024 * 1024);
 :   const int buffer_size = dis(rng_);
 :   vector original(buffer_size);
 :   vector scratch(buffer_size);
 :   GenerateRandomBytes(original.data(), buffer_size);
 :
 :   // Check all the modes
 :   AES_CIPHER_MODE modes[] = {AES_256_GCM, AES_256_CTR, 
AES_256_CFB};
 :   for (auto m : modes) {
 : EncryptionKey key;
 : memcpy(scratch.data(), original.data(), buffer_size);
 :
 : key.InitializeRandom();
 : key.SetCipherMode(m);
 :
 : ASSERT_OK(key.Encrypt(scratch.data(), buffer_size, 
scratch.data()));
 : // Check that encryption did something
 : ASSERT_NE(0, memcmp(original.data(), scratch.data(), 
buffer_size));
 : ASSERT_OK(key.Decrypt(scratch.data(), buffer_size, 
scratch.data()));
 : // Check that we get the original data back.
 : ASSERT_EQ(0, memcmp(original.data(), scratch.data(), 
buffer_size));
 :   }
 : }
These two tests are pretty similar. You could refactor the identical parts to a 
simple function, then call this function with different buffer sizes in a 
TEST_F.


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc@176
PS4, Line 176: if
nit: If


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util-test.cc@176
PS4, Line 176: runtim,
runtime


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.h@56
PS4, Line 56: enum AES_CIPHER_MODE { AES_256_CTR, AES_256_CFB, AES_256_GCM };
I think the multi-line version is more readable and more conformant to the 
Impala-style.


http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.h@86
PS4, Line 86: was support since
is supported since

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 13:

FYI: don't merge it yet, offline we agreed to postpone it to next week


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 13
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 19 Jan 2018 13:44:08 +
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-6401 : Re-apply the PPC Breakpad patches

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9077


Change subject: IMPALA-6401 : Re-apply the PPC Breakpad patches
..

IMPALA-6401 : Re-apply the PPC Breakpad patches

This change adds the updated PPC patch for Breakpad

Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
---
M buildall.sh
A 
source/breakpad/breakpad-97a98836768f8f0154f8f86e5e14c2bb7e74132e-patches/0002-Build-breakpad-97a98836-on-ppc64le.patch
2 files changed, 475 insertions(+), 1 deletion(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia40e6f9a0054b55e72350ddbb96c9ecac53b0419
Gerrit-Change-Number: 9077
Gerrit-PatchSet: 1
Gerrit-Owner: Valencia Edna Serrao 

[Impala-ASF-CR] IMPALA-6368: make test chars parallel

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9022 )

Change subject: IMPALA-6368: make test_chars parallel
..

IMPALA-6368: make test_chars parallel

Previously it had to be executed serially because it modified tables in
the functional database.

This change separates out tests that use temporary tables and runs those
in a unique_database.

Testing:
Ran locally in a loop with parallelism of 4 for a while.

Change-Id: I2f62ede90f619b8cebbb1276bab903e7555d9744
Reviewed-on: http://gerrit.cloudera.org:8080/9022
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
A testdata/workloads/functional-query/queries/QueryTest/chars-tmp-tables.test
M testdata/workloads/functional-query/queries/QueryTest/chars.test
M tests/query_test/test_chars.py
3 files changed, 286 insertions(+), 301 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2f62ede90f619b8cebbb1276bab903e7555d9744
Gerrit-Change-Number: 9022
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6368: make test chars parallel

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9022 )

Change subject: IMPALA-6368: make test_chars parallel
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f62ede90f619b8cebbb1276bab903e7555d9744
Gerrit-Change-Number: 9022
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jan 2018 09:55:51 +
Gerrit-HasComments: No

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8529 )

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

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

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

Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
Reviewed-on: http://gerrit.cloudera.org:8080/8529
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Impala Public Jenkins
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/catalog/catalog.cc
M be/src/catalog/catalog.h
M common/thrift/CatalogObjects.thrift
M common/thrift/Frontend.thrift
M common/thrift/JniCatalog.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
A fe/src/main/java/org/apache/impala/catalog/CatalogUsageMonitor.java
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
A fe/src/main/java/org/apache/impala/common/Metrics.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
A fe/src/main/java/org/apache/impala/util/TopNCache.java
A fe/src/test/java/org/apache/impala/util/TestTopNCache.java
M tests/webserver/test_web_pages.py
M www/catalog.tmpl
A www/table_metrics.tmpl
24 files changed, 1,206 insertions(+), 113 deletions(-)

Approvals:
  Dimitris Tsirogiannis: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
Gerrit-Change-Number: 8529
Gerrit-PatchSet: 7
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8529 )

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


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d407979e6d3b1a444b6b6265900b148facde9e
Gerrit-Change-Number: 8529
Gerrit-PatchSet: 6
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Jan 2018 09:25:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8950 )

Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc@689
PS3, Line 689: class DataStreamTestForImpala2931 : public DataStreamTest {
 :  protected:
 :   virtual void SetUp() {
 : DataStreamTest::SetUp();
 :   }
 :
 :   virtual void TearDown() {
 : DataStreamTest::TearDown();
 :   }
 : };
> We can do that, but then the test output shows that the test has run succes
I find this class specialization a bit misleading given it's essentialy the 
same as the base DataStreamTest. We can explicitly log in the test that it's 
skipped for KRPC if you find the output a bit unclear.

If you still prefer to do it with the current approach, please rename the class 
to DataStreamTestThriftOnly and adds a comment on what this class is for.


http://gerrit.cloudera.org:8080/#/c/8950/3/be/src/runtime/data-stream-test.cc@703
PS3, Line 703: FLAGS_datastream_service_num_deserialization_threads = 1;
 : FLAGS_datastream_service_deserialization_queue_size = 1;
> We could iterate through different config values like the link above and ru
It seems rather hard to reuse the code for exercising other cases in the KRPC 
code path given this hard coding of the flag here. Is there a better way to 
write this ?

Ideally, we can call this DataStreamTestForKRPCOnly for subset of tests which 
should be run with KRPC only.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
Gerrit-Change-Number: 8950
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 19 Jan 2018 09:09:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_tpcds_queries.py
22 files changed, 231 insertions(+), 140 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc@5017
PS4, Line 5017:   TestValue("cast(-12345.345 as double) % cast(7 as double)",
> nit: could be on same line
Unfortunately it does not fit on one line.


http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc@7618
PS4, Line 7618: TEST_F(ExprTest, DecimalOverflowCasts) {
> These will only be run with V2, but let's also run them in V1. I think we s
Done


http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2276
PS4, Line 2276: ScalarType.createDecimalType(15, 5), 
ScalarType.createDecimalType(16, 5));
> Although we have agreed upon these rules, I must still register my objectio
Haha, yes, I know what you mean :)


http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2345
PS4, Line 2345: ScalarType.createDecimalType(15, 5), 
ScalarType.createDecimalType(16, 5));
> Similarly.
Yep


http://gerrit.cloudera.org:8080/#/c/9062/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:

http://gerrit.cloudera.org:8080/#/c/9062/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@a75
PS4, Line 75:
> ??!!  did this even run before?
Not sure.. probably.. If it didn't run I assume we'd be getting an error 
because of expected results don't match the actual results


http://gerrit.cloudera.org:8080/#/c/9062/4/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:

http://gerrit.cloudera.org:8080/#/c/9062/4/tests/query_test/test_decimal_casting.py@84
PS4, Line 84: if cast_from == 'string':
> Should we have a comment that this is deprecating decimal_v1 behavior testi
We're deprecating decimal_v1 testing in many places, not just here. I don't 
think a comment about this is necessary here (unless you guys disagree).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 09:01:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
..


Patch Set 4:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@78
PS4, Line 78: string IMPALA_HOME(getenv("IMPALA_HOME"));
const static


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@90
PS4, Line 90: TLSv1.0 compatible ciphers
Are there ways to detect the TLS version supported on the platform and choose 
some TLSv1.2 ciphers too if the platform supports it ?


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@268
PS4, Line 268: RunTlsTestTemplate
What do you think about factoring line 331 to line 371 below into a single 
function and use it in the tests below and  TEST_F(RpcMgrTest, 
MultipleServices)  ?

In this way, we can exercise more than a single ping RPC in the tests below 
when TLS is enabled.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@309
PS4, Line 309:
nit: blank line


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@331
PS4, Line 331:   // Test that a service can be started, and will respond to 
requests.
 :   unique_ptr ping_impl(
 :   new PingServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(ping_impl)));
 :
 :   // Test that a second service, that verifies the RPC payload 
is not corrupted,
 :   // can be started.
 :   unique_ptr scan_mem_impl(
 :   new ScanMemServiceImpl(tls_rpc_mgr.metric_entity(), 
tls_rpc_mgr.result_tracker()));
 :   ASSERT_OK(tls_rpc_mgr.RegisterService(10, 10, 
move(scan_mem_impl)));
 :
 :   FLAGS_num_acceptor_threads = 2;
 :   FLAGS_num_reactor_threads = 10;
 :   ASSERT_OK(tls_rpc_mgr.StartServices(tls_krpc_address));
 :
 :   unique_ptr ping_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_proxy));
 :
 :   unique_ptr scan_mem_proxy;
 :   
ASSERT_OK(tls_rpc_mgr.GetProxy(tls_krpc_address, 
_mem_proxy));
 :
 :   RpcController controller;
 :   srand(0);
 :   // Randomly invoke either services to make sure a RpcMgr can 
host multiple
 :   // services at the same time.
 :   for (int i = 0; i < 100; ++i) {
 : controller.Reset();
 : if (random() % 2 == 0) {
 :   PingRequestPB request;
 :   PingResponsePB response;
 :   kudu::Status status = ping_proxy->Ping(request, , 
);
 :   ASSERT_TRUE(status.ok());
 :   ASSERT_EQ(response.int_response(), 42);
 : } else {
 :   ScanMemRequestPB request;
 :   ScanMemResponsePB response;
 :   SetupScanMemRequest(, );
 :   kudu::Status status = scan_mem_proxy->ScanMem(request, 
, );
 :   ASSERT_TRUE(status.ok());
 : }
 :   }
Please see comments above on how this may be factored into a function to be 
shared by multiple tests.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@378
PS4, Line 378:   // Misconfigure TLS with bad CA certificate.
nit: may as well move it to before line 377 as block comment to describe what 
this function tests for.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@392
PS4, Line 392:
 :   // Misconfigure TLS with a bad password command for the 
passowrd protected private key.
nit: may as well move it to before line 391 as block comment for this function.

Same for other functions.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@412
PS4, Line 412:   // Configure TLS with a password protected private key and the 
correct password for it.
nit: same here.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@430
PS4, Line 430:   // Misconfigure TLS with a bad cipher.
nit: same here


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr-test.cc@446
PS4, Line 446:   // Enable TLS with a TLSv1 compatible cipher list.
nit: same here.


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.h@100
PS4, Line 100: {
 :   }
nit: one line { }


http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8439/4/be/src/rpc/rpc-mgr.cc@93
PS4, Line 93:
nit: blank 

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..


Patch Set 19: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 19
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Jan 2018 08:10:49 +
Gerrit-HasComments: No