[Impala-ASF-CR] IMPALA-5676: avoid expensive consistency checks in BTSv2

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded a new patch set (#2).

Change subject: IMPALA-5676: avoid expensive consistency checks in BTSv2
..

IMPALA-5676: avoid expensive consistency checks in BTSv2

Doing an O(n) consistency check every time the read or write
page was advanced results in O(n^2) overall runtime.

The fix is to separate the O(1) and O(n) checks and only do the
O(n) checks if:
* The function does an an O(n) pass over the pages anyway (e.g.
  PinStream())
* The function is called only once per read or write pass over the
  stream.

This should make the cost of the checks O(n) (if we make the reasonable
assumption that PrepareForWrite(), PrepareForRead(), PinStream() and
UnpinStream() are called a bounded number of times per stream).

Testing:
Ran BufferedTupleStreamV2Test.

Change-Id: I8b380fcd0568cb73b36a490954bcd316db969ede
---
M be/src/runtime/buffered-tuple-stream-v2.cc
M be/src/runtime/buffered-tuple-stream-v2.h
2 files changed, 43 insertions(+), 28 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b380fcd0568cb73b36a490954bcd316db969ede
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-5676: avoid expensive consistency checks in BTSv2

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5676: avoid expensive consistency checks in BTSv2
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7459/1/be/src/runtime/buffered-tuple-stream-v2.h
File be/src/runtime/buffered-tuple-stream-v2.h:

Line 696:   /// has constant runtime. The Full version includes additional 
checks that are O(n)
> May help to have a one line explanation on the difference in areas in which
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b380fcd0568cb73b36a490954bcd316db969ede
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..


Patch Set 2: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 4:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 4: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


IMPALA-5686: Update a mini cluster Sentry property

The property "sentry.service.client.server.rpc-address" was recently
updated to "sentry.service.client.server.rpc-addresses", and as a
result, the mini cluster fails to start.

We fix the problem by adding the new property to the sentry-site.xml
template file. The old property is temporarily kept as a precaution.

Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Reviewed-on: http://gerrit.cloudera.org:8080/7469
Reviewed-by: Taras Bobrovytsky 
Tested-by: Impala Public Jenkins
---
M fe/src/test/resources/sentry-site.xml.template
1 file changed, 7 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Taras Bobrovytsky: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 24:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 24
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 3: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..


Patch Set 1: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 2: Code-Review+2

Forwarding the +2. Thanks for the review, Henry and Lars.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 1:

(1 comment)

Yes, I tested this change on my machine and it fixed the issue for me.

http://gerrit.cloudera.org:8080/#/c/7469/1/fe/src/test/resources/sentry-site.xml.template
File fe/src/test/resources/sentry-site.xml.template:

Line 42:  sentry.service.client.server.rpc-addresses
> Can you add a comment mentioning IMPALA-5686, saying we should remove one o
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 1: Code-Review+2

I cherry picked the change and it fixed the issue for me.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 24:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 24
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..


Patch Set 1: Code-Review+2

(1 comment)

Did this fix the issue for you?

http://gerrit.cloudera.org:8080/#/c/7469/1/fe/src/test/resources/sentry-site.xml.template
File fe/src/test/resources/sentry-site.xml.template:

Line 42:  sentry.service.client.server.rpc-addresses
Can you add a comment mentioning IMPALA-5686, saying we should remove one of 
these two properties when Sentry standardizes on one?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5686: Update a mini cluster Sentry property

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new change for review.

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

Change subject: IMPALA-5686: Update a mini cluster Sentry property
..

IMPALA-5686: Update a mini cluster Sentry property

The property "sentry.service.client.server.rpc-address" was recently
updated to "sentry.service.client.server.rpc-addresses", and as a
result, the mini cluster fails to start.

We fix the problem by adding the new property to the sentry-site.xml
template file. The old property is temporarily kept as a precaution.

Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
---
M fe/src/test/resources/sentry-site.xml.template
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I486c1ab33277f4e795be9dbbefb8d839354bf6bb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 3: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] (PREVIEW) IMPALA-5684: Optionally run be tests in sharded mode

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: (PREVIEW) IMPALA-5684: Optionally run be tests in sharded mode
..

(PREVIEW) IMPALA-5684: Optionally run be tests in sharded mode

Googletest supports sharded execution, where multiple independent
processes can run a subset of a test suite in parallel on the same
machine or different machines. This patch adds preliminary support for
doing so for some of our backend tests, where the parallelism speedup is
worth the extra overhead.

How we do this: for every backend test 'foo', add a new test target in
CMake, called 'foo_sharded'. That target executes a wrapper script,
bin/run-sharded-test.sh, which runs N copies of the test in parallel,
with the environment correctly set up to enable sharding. You can run a
sharded test like this:

ctest -R "foo_sharded"

Then, in bin/run-backend-tests.sh, keep a whitelist of tests that can be
successfully run in sharded mode (i.e. there are no known conflicts from
running multiple copies at once, and the speedup is worth the
overhead). Use that list to run ctest twice, once in serial mode with no
sharded tests (using -E to exclude those tests) and once in sharded mode
running only the whitelisted sharded tests.

Each shard process logs to its own directory 'foo_test_shard_N' under
the usual logs/be_tests/ root.

On my desktop machine, this improves expr-test runtime from ~10 minutes
to about 5 minutes. More importantly, this opens up more opportunity for
easy latency improvements if we break the large test cases into smaller
ones that are parallelisable.

Change-Id: I865db25b07728f3886133316ded5122c60490967
---
M be/CMakeLists.txt
M bin/run-backend-tests.sh
A bin/run-sharded-test.sh
3 files changed, 97 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I865db25b07728f3886133316ded5122c60490967
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[David Knupp (Code Review)]

David Knupp has posted comments on this change.

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: David Knupp 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4868: Disable flaky TestRequestPoolService test

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-4868: Disable flaky TestRequestPoolService test
..

IMPALA-4868: Disable flaky TestRequestPoolService test

Disables a test that seemed to get flaky recently, perhaps
related to testing with Java 8 or maybe even changes in YARN
(which get used by RequestPoolService).

Since we're not changing this code right now, let's disable
this test to unblock builds. Keeping the JIRA open to track
a better solution.

Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
---
M fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I616961457cd48d31d618c8b58f5279b89d3cdcd6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-5625: write profile when query times out

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-5625: write profile when query times out
..


Patch Set 2:

(8 comments)

Thanks for this patch, I think this will be a great improvement.

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

PS2, Line 7: IMPALA-5625: write profile when query times out
Please amend to say this is for the stress test, and cover general failure.

  IMPALA-5625: stress test: write profile when queries fail


PS2, Line 22: the the
"the the"


PS2, Line 25: Testing:
Great testing, thanks.


http://gerrit.cloudera.org:8080/#/c/7376/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS2, Line 700:   raise Exception(dedent("""Result hash mismatch; 
expected {0}, got {1}
 :  Query ID: {2}
 :  Query: 
{3}""".format(query.result_hash,
 :   
report.result_hash,
 :   
report.query_id, query.sql)))
For complicated strings like this, prefer format() with a kwargs-style set of 
arguments. It makes the string more readable.

  "{descriptive_arg} ...".format(descriptive_arg=val, ...)

The level of complication is subjective, but multiple args + multiple lines is 
probably enough to warrant it.


PS2, Line 708: "Query unexpectedly timed out. Query ID: 
{0}".format(report.query_id))
This needs to be indented 2 more spaces to satisfy flake8.


PS2, Line 818: self.results_dir = gettempdir()
I know this isn't a functional change, but check whether this gettempdir() call 
is needed and remove if you can.


PS2, Line 1668: get_profile
Would set_profile be a more descriptive, accurate name?


PS2, Line 1670:   Producing a query profile can be somewhat expensive. A v-tune 
profile of
  :   impalad showed 10% of cpu time spent generating query 
profiles.
Thanks for preserving this comment. Since we could be collecting more profiles 
now, have you examined CPU or other performance differences? Do we need to be 
able to disable profile collection, or collect profiles based on sampling 
instead of always, or avoid collecting more than 1 profile at a time?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Mulder 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


IMPALA-5670: Misc. tidying of ExecEnv

* Remove duplicated c'tor code for ExecEnv() and delegate it onto
  6-argument c'tor.
* Make some RM-related flags hidden
* Remove unused and untested 'use_statestore' branch in ExecEnv which
  created a scheduler with static hosts rather than those sent by the
  statestore.
* Remove corresponding scheduler c'tor to handle the above path.
* Remove unused 'use_statestore' parameter from
  InProcessImpalaServer::StartAsBackendOnly() and
  InProcessImpalaServer::StartWithClientServers().

Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Reviewed-on: http://gerrit.cloudera.org:8080/7445
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M be/src/exprs/expr-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/testutil/mini-impala-cluster.cc
M docs/topics/impala_config_options.xml
M docs/topics/impala_upgrading.xml
12 files changed, 71 insertions(+), 178 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 6: Code-Review+2

Fix typo, carry +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

[anujphadke (Code Review)]

anujphadke has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7393/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 1023: for (int j = 0; j < build_rows->num_rows(); ++j) {
> We still need the check in the outer loop in case there are many matched ro
I thought you implied to move this inside. 
Makes sense we could be  stuck in the outer loop as well as the inner one.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

[anujphadke (Code Review)]

Hello Thomas Tauber-Marshall,

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

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

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

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..

IMPALA-5586: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe() and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 16 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts

The -use_local_tz_for_unix_timestamp_conversion flag exists
to specify if TIMESTAMPs should be interpreted as localtime
or UTC when converting to/from Unix time via builtins:
  from_unixtime(bigint unixtime)
  unix_timestamp(string datetime[, ...])
  unix_timestamp(timestamp datetime)

However, the KuduScanner was calling into code that, when
the gflag above was set, interpreted Unix times as local
time.  Unfortunately the write path (KuduTableSink) and some
FE TIMESTAMP code (see KuduUtil.java) did not have this
behavior, i.e. we were handling the gflag inconsistently.

Tests:
* Adds a custom cluster test to run Kudu test cases with
  -use_local_tz_for_unix_timestamp_conversion.
* Adds tests for the new builtin
  unix_micros_to_utc_timestamp() which run in a custom
  cluster test (added test_local_tz_conversion.py) as well
  as in the regular tests (added to test_exprs.py).

Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Reviewed-on: http://gerrit.cloudera.org:8080/7311
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exec/kudu-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/catalog/KuduColumn.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
A 
testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_kudu.py
A tests/custom_cluster/test_local_tz_conversion.py
M tests/query_test/test_exprs.py
M tests/query_test/test_kudu.py
15 files changed, 146 insertions(+), 16 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 6: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 2: Code-Review+1

(1 comment)

Looks good, this should be a big improvement. There's an additional 
nice-to-have but I won't block this if it's non-trivial.

http://gerrit.cloudera.org:8080/#/c/7449/2/be/src/common/status.cc
File be/src/common/status.cc:

Line 48: Status::Status(TErrorCode::type code)
It would be nice to have versions of Status::Expected() that took the 
predefined error codes, since we should be encouraging use of those instead of 
unstructured strings.

It would add a lot of boilerplate to stamp out all the variants manually, but 
it looks it's possible to forward multiple arguments in C++11: 
https://stackoverflow.com/a/2821244. We could probably get ride of the below 
boilerplate too, sicne they are all just forwarding the arguments to ErrorMsg()

I'm ok with the current code if that isn't easily doable but it would be nice 
to do the cleanup.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 2: Verified-1

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 2: Code-Review+1

Looks good to me. Lets give Tim and/or Sailesh another chance to comment if 
they'd like to.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has uploaded a new patch set (#2).

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..

IMPALA-5275: Avoid printing status stack trace on hot paths

Currently, creation of a Status object (non-OK and non-EXPECTED)
prints the stack trace to the log. Fetching the stack trace takes
a large chunk of CPU time close to 130ms and results in a significant
perf hit when encountered on hot paths.
Five such hot paths were identified and the following changes were
made to fix it:

1. In ImpalaServer::GetExecSummary(), create Status() without holding
the query_log_lock_.
2, 3 and 4. In impala::DeserializeThriftMsg<>(),
PartitionedAggregationNode::CodegenUpdateTuple() and
HdfsScanner::CodegenWriteCompleteTuple, use Status::Expected where
appropriate.
5. In Status::MemLimitExceeded(), create Status object without
printing stacktrace

Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
---
M be/src/common/status.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/rpc/thrift-util.h
M be/src/service/impala-server.cc
5 files changed, 28 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6898/4/testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-catalogd.test
File 
testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-catalogd.test:

Line 1: # TODO: enable this once we have a way to invalidate kudu clients 
(IMPALA-5685)
A few notes here:
- Moved to the top as the test file parser ignores 'headers'. Seemed like the 
cleanest way to disable it.
- I renamed the table here as the test previously wasn't actually testing what 
its supposed to - it was just returning the cached failed table load from the 
first query.
- The other tests in the file still work as the second and third tests are 
related to table creation situations, where the table's metadata won't have 
been loaded by the previous query.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

Hello Impala Public Jenkins, Matthew Jacobs,

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

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

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

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..

IMPALA-5167: Reduce the number of Kudu clients created (FE)

Creating Kudu clients is very expensive as each will fetch
metadata from the Kudu master, so we should minimize the
number of Kudu clients that get created.

This patch stores a map from Kudu master addressed to Kudu
clients in KuduUtil to be used across the FE and catalog.
Another patch has already addressed the BE.

Future work will consider providing a way to invalidate
the stored Kudu clients in case something goes wrong
(IMPALA-5685)

This relies on two changes on the Kudu side: one that clears
non-covered range entries from the client's cache on table
open (d07ecd6ded01201c912d2e336611a6a941f48d98), and one
that automatically refreshes auth tokens when they expire
(603c1578c78c0377ffafdd9c427ebfd8a206bda3).

This patch disables some tests that no longer work as
they relied on Kudu metadata loading operations timing out,
but since we're reusing clients the metadata is already
loaded when the test is run.

Testing:
- Ran a stress test on a 10 node cluster: scan of a small
  Kudu table, 1000 concurrent queries, load on the Kudu
  master was reduced signficantly, from ~50% cpu to ~5%.
  (with the BE changes included)
- Ran the Kudu e2e tests.
- Manually ran a test with concurrent INSERTs and
  'ALTER TABLE ADD PARTITION' (which is affected by the
  Kudu side change mentiond above) and verified
  correctness.

Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
---
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M 
testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-catalogd.test
M 
testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-impalad.test
6 files changed, 55 insertions(+), 31 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[anujphadke (Code Review)]

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7332/7/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS7, Line 105: " 
> technically this is still mixed case, no?
Done.


Line 168: try:
> This line should be outside of the try loop, otherwise role_name isn't in s
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[anujphadke (Code Review)]

Hello Bharath Vissapragada, Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 187 insertions(+), 93 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Bikramjeet Vig (Code Review)]

Hello Impala Public Jenkins, Henry Robinson, Matthew Jacobs,

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

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

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

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..

IMPALA-4276: Profile displays non-default query options set by planner

Fix to populate the non-default query options set by planner in the
runtime profile.

Added a corresponding test case.

Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
---
M be/src/service/client-request-state.cc
M tests/custom_cluster/test_admission_controller.py
M tests/query_test/test_observability.py
3 files changed, 21 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 7:

(2 comments)

Thanks! 2 small things and otherwise I'm good to go. Let's make sure Michael 
Brown is also OK with it.

http://gerrit.cloudera.org:8080/#/c/7332/7/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS7, Line 105: _y
technically this is still mixed case, no?


Line 168:   role_name = "test_role_" + get_random_id(5)
This line should be outside of the try loop, otherwise role_name isn't in scope 
at l192. Also, if it fails, it doesn't need to execute the finally.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[anujphadke (Code Review)]

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7332/6/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS6, Line 104:   db_name = "test_role_privilege_case_x_" + ge
> Why use a random suffix here but not in test_grant_revoke?
Done.
I  did not look much into that test since this patch only adds 
test_role_privilege_case.
Thanks for pointing that out.


PS6, Line 105:   db_name_upper_case = "TEST_ROLE_PRIVILEGE_CASE_y_" + 
get_random_id(5)
 :   db_name_mixed_case = "TesT_Role_PRIVIlege_case
> Maybe use TEST_GRANT_REVOKE and Test_GraNt_revoke or similar for prefixes?
Done


Line 138:   assert any(db_name_mixed_case.lower() in x for x in result.data)
> Yeah, most of these fit within 90 chars on 1 line. For the ones that don't,
Done


PS6, Line 195: 
> Not your change, but could you remove this semi-colon?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[anujphadke (Code Review)]

Hello Bharath Vissapragada, Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 187 insertions(+), 93 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3:

> So the GVO failed because there's a custom cluster test that runs
 > impala with a very short (1ms) timeout for Kudu operations and then
 > runs two queries, each of which it expects to timeout while
 > retrieving table metadata.
 > 
 > Since we reuse clients now, the metadata ends up being fetched by
 > the first query (even though the operation times out, it apparently
 > still completes afterwards) and then the second query doesn't time
 > out since the metadata is already preset (both tests currently
 > touch the same table, but changing the second one to a different
 > table doesn't seem to fix the problem, presumably the Kudu client
 > loads more tables than explicitly requested).
 > 
 > Some possible solutions:
 > - Only run one of the queries. This would cause some coverage to be
 > lost (we could replace it by adding a test that checks for timeout
 > behavior in the BE, which I don't think currently exists, and would
 > work since the BE and FE still use different clients)
 > - Run both of the tests in separate custom_cluster tests.
 > custom_cluster tests are expensive (this one runs for ~20s) so it
 > seems like a lot of test overhead for little benefit.
 > - Provide some way to invalidate the Kudu clients, eg. have
 > 'invalidate metadata' clear the list of clients in KuduUtil. We may
 > want to do something like this anyways since in the current version
 > of the patch once a KuduClient is created for a particular Kudu
 > master, that client will live on as long as the impalad does, and
 > if something goes wrong (eg. the automatic token refresh that the
 > Kudu team implemented doesn't work for some reason) you're
 > basically screwed and won't be able to do anything with that Kudu
 > master without restarting the impalad.

Good catch, yes that seems like the test won't work after your change.

Let's do this:
1) Change the tests (I think you'll have to update both kudu-timeouts-impalad 
and kudu-timeouts-catalogd) to only run the first for now. I agree it's not a 
huge amount of lost coverage. Just comment out the rest and leave a TODO to add 
them back, w/ a reference to the JIRA in #2.
2) File a JIRA to provide some way to invalidate the Kudu clients. It's a bit 
different from HMS metadata since it's not really per table, it's per Kudu 
server. I'm not sure yet if this is something that will be worthwhile to expose 
for general use (e.g. a well documented stmt like 'invalidate metadata'), or if 
we should try to do something that would be more covert, e.g. a web endpoint 
(hacky).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3:

So the GVO failed because there's a custom cluster test that runs impala with a 
very short (1ms) timeout for Kudu operations and then runs two queries, each of 
which it expects to timeout while retrieving table metadata.

Since we reuse clients now, the metadata ends up being fetched by the first 
query (even though the operation times out, it apparently still completes 
afterwards) and then the second query doesn't time out since the metadata is 
already preset (both tests currently touch the same table, but changing the 
second one to a different table doesn't seem to fix the problem, presumably the 
Kudu client loads more tables than explicitly requested).

Some possible solutions:
- Only run one of the queries. This would cause some coverage to be lost (we 
could replace it by adding a test that checks for timeout behavior in the BE, 
which I don't think currently exists, and would work since the BE and FE still 
use different clients)
- Run both of the tests in separate custom_cluster tests. custom_cluster tests 
are expensive (this one runs for ~20s) so it seems like a lot of test overhead 
for little benefit.
- Provide some way to invalidate the Kudu clients, eg. have 'invalidate 
metadata' clear the list of clients in KuduUtil. We may want to do something 
like this anyways since in the current version of the patch once a KuduClient 
is created for a particular Kudu master, that client will live on as long as 
the impalad does, and if something goes wrong (eg. the automatic token refresh 
that the Kudu team implemented doesn't work for some reason) you're basically 
screwed and won't be able to do anything with that Kudu master without 
restarting the impalad.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 5:

There's no rush, we can leave it open, just wanted to clarify what the state of 
it was.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 5:

> Did this get lost in the shuffle? I'm trying to pare down my review
 > inbox a bit :)

It's still on my list of things to pick back up, but has been pushed out by 
more important stuff. Apologies for the delay. Should we just abandon it?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Michael Ho, Sailesh Mukil,

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

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

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

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..

IMPALA-5670: Misc. tidying of ExecEnv

* Remove duplicated c'tor code for ExecEnv() and delegate it onto
  6-argument c'tor.
* Make some RM-related flags hidden
* Remove unused and untested 'use_statestore' branch in ExecEnv which
  created a scheduler with static hosts rather than those sent by the
  statestore.
* Remove corresponding scheduler c'tor to handle the above path.
* Remove unused 'use_statestore' parameter from
  InProcessImpalaServer::StartAsBackendOnly() and
  InProcessImpalaServer::StartWithClientServers().

Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
---
M be/src/exprs/expr-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/testutil/mini-impala-cluster.cc
M docs/topics/impala_config_options.xml
M docs/topics/impala_upgrading.xml
12 files changed, 71 insertions(+), 178 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 6:

> (4 comments)
 > 
 > None of these comments are must-dos, just things I've noticed.

Anuj, were you planning on making these changes now? I think some of them are 
trivial and worth making, it seems OK to leave changing the other test cases as 
Michael suggested for a future change (add a TODO).

Can you please file a JIRA about the bug you found with test_role_update ?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..


Patch Set 34: Code-Review+2

Updated test_explain with a minor change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
Gerrit-PatchSet: 34
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

[Tim Armstrong (Code Review)]

Hello Dan Hecht,

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

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

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

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..

IMPALA-4674: Part 2: port backend exec to BufferPool

Always create global BufferPool at startup using 80% of memory and
limit reservations to 80% of query memory (same as BufferedBlockMgr).
The query's initial reservation is computed in the planner, claimed
centrally (managed by the InitialReservations class) and distributed
to query operators from there.

min_spillable_buffer_size and default_spillable_buffer_size query
options control the buffer size that the planner selects for
spilling operators.

Port ExecNodes to use BufferPool:
  * Each ExecNode has to claim its reservation during Open()
  * Port Sorter to use BufferPool.
  * Switch from BufferedTupleStream to BufferedTupleStreamV2
  * Port HashTable to use BufferPool via a Suballocator.

This also makes PAGG memory consumption more efficient (avoid wasting buffers)
and improve the spilling algorithm:
* Allow preaggs to execute with 0 reservation - if streams and hash tables
  cannot be allocated, it will pass through rows.
* Halve the buffer requirement for spilling aggs - avoid allocating
  buffers for aggregated and unaggregated streams simultaneously.
* Rebuild spilled partitions instead of repartitioning (IMPALA-2708)

TODO in follow-up patches:
* Rename BufferedTupleStreamV2 to BufferedTupleStream
* Implement max_row_size query option.

Testing:
* Updated tests to reflect new memory requirements

Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/analytic-eval-node.cc
M be/src/exec/analytic-eval-node.h
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hash-table-test.cc
M be/src/exec/hash-table.cc
M be/src/exec/hash-table.h
M be/src/exec/hash-table.inline.h
M be/src/exec/nested-loop-join-builder.cc
M be/src/exec/partitioned-aggregation-node-ir.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/partitioned-hash-join-builder-ir.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node-ir.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/exec/partitioned-hash-join-node.inline.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/CMakeLists.txt
D be/src/runtime/buffered-block-mgr-test.cc
D be/src/runtime/buffered-block-mgr.cc
D be/src/runtime/buffered-block-mgr.h
D be/src/runtime/buffered-tuple-stream-test.cc
D be/src/runtime/buffered-tuple-stream.cc
D be/src/runtime/buffered-tuple-stream.h
D be/src/runtime/buffered-tuple-stream.inline.h
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.h
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/initial-reservations.cc
A be/src/runtime/initial-reservations.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M be/src/runtime/test-env.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/service/client-request-state.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.h
M be/src/util/static-asserts.cc
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/DataStreamSink.java
M fe/src/main/java/org/apache/impala/planner/EmptySetNode.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M 

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7393/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 1023:   // This loop may run for a long time. Periodically check for 
cancellation.
We still need the check in the outer loop in case there are many matched rows.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 5:

Did this get lost in the shuffle? I'm trying to pare down my review inbox a bit 
:)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5317: add DATE TRUNC() function

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5317: add DATE_TRUNC() function
..


Patch Set 1:

Any updates here? Can I help move this along somehow?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I953ba006cbb166dcc78e8c0c12dfbf70f093b584
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: sandeep akinapelli 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

Yes I think never printing stacktraces for MemLimitExceeded() makes sense. We 
have other diagnostics and logging for MemLimitExceeded() that is more useful.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7445/5/be/src/testutil/mini-impala-cluster.cc
File be/src/testutil/mini-impala-cluster.cc:

Line 80:   
ABORT_IF_ERROR(impala_servers[i]->StartWithClientServers(beeswax_port, 
hs2_port);
> Missing )
Yeah, I saw that, thanks.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7445/5/be/src/testutil/mini-impala-cluster.cc
File be/src/testutil/mini-impala-cluster.cc:

Line 80:   
ABORT_IF_ERROR(impala_servers[i]->StartWithClientServers(beeswax_port, 
hs2_port);
Missing )


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


IMPALA-4925: Cancel finstance if query has finished

This patch is a partial fix for the issue where an finst would not
detect that it should cancel if the query limit had not been hit. It
changes the UpdateExecStatus() RPC to return a cancelled status to an
finst if the query has finished because it hit a limit.

For certain queries, this allows them to finish much more quickly than
they otherwise would. However, there's still a few-second delay for the
finst to pick up the cancellation signal, because there
UpdateExecStatus() RPC is only called every few seconds.

A complete fix would also call CancelInternal() when returned_all_results_
was set to true. That would be a much larger change. The improvement
here is to bound the delay between query completion and fragment
teardown to a few seconds.

Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Reviewed-on: http://gerrit.cloudera.org:8080/5987
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M be/src/runtime/coordinator.cc
M tests/query_test/test_lifecycle.py
2 files changed, 30 insertions(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 6:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/880/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7311/4/tests/custom_cluster/test_local_tz_conversion.py
File tests/custom_cluster/test_local_tz_conversion.py:

Line 36: self.run_test_case('QueryTest/utc-timestamp-functions', vector)
> We should make sure to run this with the same test matrix as test_exprs - i
Done


http://gerrit.cloudera.org:8080/#/c/7311/4/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:

Line 150: class TestUtcTimestampFunctions(ImpalaTestSuite):
> Should also run this with the same test dimensions as TestExprs
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new patch set (#6).

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..

IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts

The -use_local_tz_for_unix_timestamp_conversion flag exists
to specify if TIMESTAMPs should be interpreted as localtime
or UTC when converting to/from Unix time via builtins:
  from_unixtime(bigint unixtime)
  unix_timestamp(string datetime[, ...])
  unix_timestamp(timestamp datetime)

However, the KuduScanner was calling into code that, when
the gflag above was set, interpreted Unix times as local
time.  Unfortunately the write path (KuduTableSink) and some
FE TIMESTAMP code (see KuduUtil.java) did not have this
behavior, i.e. we were handling the gflag inconsistently.

Tests:
* Adds a custom cluster test to run Kudu test cases with
  -use_local_tz_for_unix_timestamp_conversion.
* Adds tests for the new builtin
  unix_micros_to_utc_timestamp() which run in a custom
  cluster test (added test_local_tz_conversion.py) as well
  as in the regular tests (added to test_exprs.py).

Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
---
M be/src/exec/kudu-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/catalog/KuduColumn.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
A 
testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_kudu.py
A tests/custom_cluster/test_local_tz_conversion.py
M tests/query_test/test_exprs.py
M tests/query_test/test_kudu.py
15 files changed, 146 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new patch set (#5).

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..

IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts

The -use_local_tz_for_unix_timestamp_conversion flag exists
to specify if TIMESTAMPs should be interpreted as localtime
or UTC when converting to/from Unix time via builtins:
  from_unixtime(bigint unixtime)
  unix_timestamp(string datetime[, ...])
  unix_timestamp(timestamp datetime)

However, the KuduScanner was calling into code that, when
the gflag above was set, interpreted Unix times as local
time.  Unfortunately the write path (KuduTableSink) and some
FE TIMESTAMP code (see KuduUtil.java) did not have this
behavior, i.e. we were handling the gflag inconsistently.

Tests:
* Adds a custom cluster test to run Kudu test cases with
  -use_local_tz_for_unix_timestamp_conversion.
* Adds tests for the new builtin
  unix_micros_to_utc_timestamp() which run in a custom
  cluster test (added test_local_tz_conversion.py) as well
  as in the regular tests (added to test_exprs.py).

Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
---
M be/src/exec/kudu-scanner.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/catalog/KuduColumn.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
A 
testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_kudu.py
A tests/custom_cluster/test_local_tz_conversion.py
M tests/query_test/test_exprs.py
M tests/query_test/test_kudu.py
15 files changed, 137 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4674: add new message to stress test

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged.

Change subject: IMPALA-4674: add new message to stress test
..


IMPALA-4674: add new message to stress test

The main IMPALA-4674 commit adds a new OOM failure mode where the query
can't get its minimum reservation during query startup. The new message
includes the string "failed to get minimum memory reservation" along
with some additional context.

Testing:
Ran a stress test using the modified script. Verified it treats failure
to get minimum reservation in the same way as mem limit exceeded.

Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
Reviewed-on: http://gerrit.cloudera.org:8080/7458
Reviewed-by: Michael Brown 
Tested-by: Tim Armstrong 
---
M tests/stress/concurrent_select.py
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Michael Brown: Looks good to me, approved
  Tim Armstrong: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4674: add new message to stress test

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: add new message to stress test
..


Patch Set 1: Verified+1

This isn't exercised by the pre-merge tests so will manually verify.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 2:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/879/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5625: write profile when query times out

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-5625: write profile when query times out
..


Patch Set 2:

> I replied to them with "Done", but they show up as Draft, so I
> wonder if you don't see them.

I see them now. You have to do a top-level reply to publish all the inline 
comments. (This workflow is different than Github's, in which each comment 
appears individually.)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Mulder 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5625: write profile when query times out

[Matthew Mulder (Code Review)]

Matthew Mulder has posted comments on this change.

Change subject: IMPALA-5625: write profile when query times out
..


Patch Set 1:

(8 comments)

> Matt, you uploaded a new patch set, but please reply to the inline
 > comments with either "done" or some other explanation.

I replied to them with "Done", but they show up as Draft, so I wonder if you 
don't see them.

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

Line 22: 
> We typically have a testing done section.
Done


http://gerrit.cloudera.org:8080/#/c/7376/1/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS1, Line 691: "{0}"
> Please add query ID: to the string here.
Done


Line 699:   raise Exception(
> Can you also add support for saving profiles when incorrect results occur?
Done


PS1, Line 704:  {0}
> Please add "query ID" to the string.
Done


PS1, Line 748: if not (report.profile and report.query_id):
 :   return
> Do you know often this path is executed?
This is executed every time the profile is attempted to be written because of a 
query failure. This check cowardly refuses to write the profile if there is no 
profile or if we fail to get the query id. This could happen if there's a 
communication error or some other serious problem.


Line 865:   LOG.debug("Query id is %s", report.query_id)
> Should the comma be a % sign?
The prior version uses a comma, and the test output looks correct. It looks 
like all of the LOG.debug function calls use a comma. It would probably work to 
use a %, but it's better to follow the current style without good cause to 
change.


Line 871: report.profile = cursor.get_profile()
> Do we want to set should_cancel here since time is exceeded?
should_cancel indicates whether the query timeout was purposely shortened to 
induce a cancelation as part of the stress procedure. See line 642. Looks like 
I should have documented this in the function doc.


PS1, Line 1704: "--result-hash-log-dir"
> Now that this contains profiles, too, should this be renamed?
Yeah, good point. I should update the help text as well. How about 
"--results-dir" which is slightly more generic, or "--artifacts-dir"? I don't 
want to be too specific with something like "--results-and-profiles-dir" 
because then it would have to be changed again when a new kind of artifact, say 
CSV files or charts, are added.

Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Mulder 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..

IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

Kudu tables did not treat some table properties correctly.

Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke_kudu.test
M tests/authorization/test_grant_revoke.py
6 files changed, 170 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-5625: write profile when query times out

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-5625: write profile when query times out
..


Patch Set 2:

Matt, you uploaded a new patch set, but please reply to the inline comments 
with either "done" or some other explanation.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Mulder 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4674: add new message to stress test

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-4674: add new message to stress test
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Attila Jeges (Code Review)]

Attila Jeges has posted comments on this change.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


Patch Set 4:

> Patch Set 3: Code-Review+2

Thanks for reviewing it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE

[Attila Jeges (Code Review)]

Attila Jeges has abandoned this change.

Change subject: IMPALA-3548: Prune runtime filters based on query options in 
the FE
..


Abandoned

It was decided that another approach should be taken to implement this feature. 
I'm abandoning this change.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I422e48847428cab9887aee899fed47a8de95c323
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change.

Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h
File be/src/gutil/hash/hash.h:

Line 278
> I looked at your comment. I did not respond yet because it is premature for
I found the specialized code from 218 to 328 is unused. My conclusion is the 
code can be removed because of the reason below. I believe there is no logical 
issue on my change.

1. hash functions are only used in hash_map/hash_set
(Checking with the command: git grep __gnu_cxx src)
2. hash_map/hash_set => unordered_map/unordered_set
The key type is only 'string'. std::hash is specialized in C++11.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 5: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/878/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 24: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/876/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 24
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


IMPALA-5407: Fix crash in HdfsSequenceTableWriter

The following use of sequence file writer can lead to a crash:
> set compression_codec=gzip;
> set seq_compression_mode=record;
> set allow_unsupported_formats=1;
> create table seq_tbl like tbl stored as sequencefile;
> insert into seq_tbl select * from tbl;

This fix removes the MemPool::FreeAll() call from
HdfsSequenceTableWriter::Flush(). Freeing the memory pool in Flush()
is incorrect because a memory pool buffer is cached by the compressor
in the table writer which isn't reset across calls to Flush().

If the file that is being written is big enough,
HdfsSequenceTableWriter::AppendRows() will call Flush() multiple
times causing memory corruption.

Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Reviewed-on: http://gerrit.cloudera.org:8080/7394
Reviewed-by: Michael Ho 
Tested-by: Impala Public Jenkins
---
M be/src/exec/hdfs-sequence-table-writer.cc
M testdata/workloads/functional-query/queries/QueryTest/seq-writer.test
2 files changed, 18 insertions(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No