[Impala-ASF-CR] IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib039d0102878f1c05470371f581cb258287b9bc0
Gerrit-Change-Number: 7388
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 04 Jan 2018 07:21:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib039d0102878f1c05470371f581cb258287b9bc0
Gerrit-Change-Number: 7388
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 04 Jan 2018 07:27:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue
..

IMPALA-4978 / IMPALA-5631: [DOCS] Add FQDN known issue

Because there was no obvious subcategory of known issues to
put this one in, I made a new subcategory 'startup issues'.

Change-Id: Ib039d0102878f1c05470371f581cb258287b9bc0
Reviewed-on: http://gerrit.cloudera.org:8080/7388
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_known_issues.xml
1 file changed, 48 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib039d0102878f1c05470371f581cb258287b9bc0
Gerrit-Change-Number: 7388
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..


Patch Set 3:

(2 comments)

The build has run successfully.  
https://jenkins.impala.io/job/gerrit-verify-dryrun-external/55/

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2231
PS1, Line 2231:   private boolean alterTableSetFileFormat(Table tbl,
> I guess you might use the code in this function. I think there are some red
I had originally tried to reuse this code but the file format is a different 
set of parameters that the serde info as part of the storage descriptor and I 
thought trying to combine them would be confusing.


http://gerrit.cloudera.org:8080/#/c/8928/1/tests/query_test/test_delimited_text.py
File tests/query_test/test_delimited_text.py:

http://gerrit.cloudera.org:8080/#/c/8928/1/tests/query_test/test_delimited_text.py@164
PS1, Line 164:   def test_delimited_text_partitioned(self, vector, 
unique_database):
> This test is very similar to test_delimited_text_alter. Do you refactor the
This test is specific to data that is partitioned.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 3
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Thu, 04 Jan 2018 03:27:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 02:30:47 +
Gerrit-HasComments: No

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

[Pranay Singh (Code Review)]

Pranay Singh has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8936


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

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

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

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

Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
---
M be/src/exec/hdfs-rcfile-scanner.cc
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/util/decompress.cc
M tests/query_test/test_scanners_fuzz.py
4 files changed, 25 insertions(+), 7 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 8936
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81
Gerrit-Change-Number: 8854
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 01:23:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'
..

IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'

This patch fixes several issues related to the min/max aggregate
functions and their handling of 'nan' and 'inf':
- Previously, if 'inf' or '-inf' was the only value for the min/max
  and codegen was being used, the result would be incorrect. This
  occurred, for example in the case of 'inf' and 'min', because we
  set an initial value of numeric_limits::max, which is less than
  'inf', so the returned min was numeric_limits::max when it should be
  'inf'. The fix is to set the initial value to
  numeric_limits::infinity.
- Previously, if one of the values was 'nan', the result of min/max
  was non-deterministic depending on the order the values were
  evaluated in. This occurs because 'nan' < or > 'any value' is always
  false, so if the first value added was 'nan', all other comparisons
  would be false and 'nan' would be returned, whereas if the first
  value wasn't 'nan' then the 'nan' wouldn't be returned. The fix is
  to treat 'nan' specially and to always return 'nan' if there is a
  single 'nan' value.

Testing:
- Added e2e tests for both scenarios, as well as adding a little extra
  nan/inf coverage for other aggregate functions.

Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81
Reviewed-on: http://gerrit.cloudera.org:8080/8854
Reviewed-by: Bikramjeet Vig 
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/expr-value.h
M testdata/workloads/functional-query/queries/QueryTest/aggregation.test
6 files changed, 97 insertions(+), 35 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81
Gerrit-Change-Number: 8854
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6362: avoid Reservation/MemTracker deadlock

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8933 )

Change subject: IMPALA-6362: avoid Reservation/MemTracker deadlock
..


Patch Set 1:

(1 comment)

fix looks good to me.
just a small ques that I ran into while trying to walk through the code path 
again that caused this issue.

http://gerrit.cloudera.org:8080/#/c/8933/1/be/src/runtime/bufferpool/reservation-tracker.cc
File be/src/runtime/bufferpool/reservation-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/8933/1/be/src/runtime/bufferpool/reservation-tracker.cc@209
PS1, Line 209: parent_
I think it is possible to get to this point and have parent_ == nullptr

steps:
if parent_ == nullptr, L195 sets granted = true, then  
TryConsumeFromMemTracker(reservation_increase) fails at L230



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
Gerrit-Change-Number: 8933
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 01:03:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3282: Adds regexp escape built-in function

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3282: Adds regexp_escape built-in function
..


Patch Set 2:

(1 comment)

I didn't take a close look at the code - maybe Tianyi can take a look, but it 
would be good to add extra tests to make sure that it interacts with other 
regexpr functions in the expected way.

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

http://gerrit.cloudera.org:8080/#/c/8900/2/be/src/exprs/expr-test.cc@4185
PS2, Line 4185:   
TestStringValue("regexp_escape('a.b*cd+e?f^g[h]i(j)k{l}m$n!o=p:q-r#s\nt\ru\tv\vw
 x"
We should also directly test that the escaping is correct for our other regexp 
functions, by testing some calls like regexp_*(regexp_escape(...), ...)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c3e0ded26f6eb20794c38b75be9b25cd111e4b
Gerrit-Change-Number: 8900
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 00:55:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6362: avoid Reservation/MemTracker deadlock

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6362: avoid Reservation/MemTracker deadlock
..


Patch Set 1: Code-Review+1

Fix makes sense to me. Not sure if there is a way to add a test for it ?  I'll 
let Bikram + 2 it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
Gerrit-Change-Number: 8933
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 00:50:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6348: Redact only sensitive fields in runtime profiles

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8934


Change subject: IMPALA-6348: Redact only sensitive fields in runtime profiles
..

IMPALA-6348: Redact only sensitive fields in runtime profiles

Without this patch, redaction is applied to every field in the
runtime profile. This approach has an undesired side effect when
Kerberos auth + email redaction is in place.

Since the redaction applies to every field, even principals
(from Connected/Delegated User fields) are redacted, as the Kerberos
principal format generally pattern matches with an email redactor
template.

This is particularly problematic for monitoring tools that consume
runtime profiles and use these fields to group the queries by user.

This patch fixes the problem by redacting only the following sensitive
fields.

- Query Statement
- Error logs (since they can contain column references etc.)
- Query Status
- Query Plan

Other fields in the runtime profile are left unredacted.

Change-Id: Iae3b6726009bf458a7ec73131e5d659b12ab73cf
---
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M tests/custom_cluster/test_redaction.py
5 files changed, 47 insertions(+), 12 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae3b6726009bf458a7ec73131e5d659b12ab73cf
Gerrit-Change-Number: 8934
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada 

[Impala-ASF-CR] IMPALA-6362: avoid Reservation/MemTracker deadlock

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6362: avoid Reservation/MemTracker deadlock
..


Patch Set 1:

Yeah that would help. Bikram and I thought about the lock ordering but missed 
that ReservationTracker::lock_ could be acquired when refreshing a consumption 
metric.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
Gerrit-Change-Number: 8933
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 04 Jan 2018 00:02:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6362: avoid Reservation/MemTracker deadlock

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6362: avoid Reservation/MemTracker deadlock
..


Patch Set 1:

Meta comment: if we have lock ranking to enforce lock ordering, will this kind 
of bug less likely to happen ? We ran into similar lock ordering issue in 
IMPALA-6346


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
Gerrit-Change-Number: 8933
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 03 Jan 2018 23:59:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6362: avoid Reservation/MemTracker deadlock

[Tim Armstrong (Code Review)]

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


Change subject: IMPALA-6362: avoid Reservation/MemTracker deadlock
..

IMPALA-6362: avoid Reservation/MemTracker deadlock

Avoid the circular dependency between ReservationTracker::lock_ and
MemTracker::child_trackers_lock_ by not acquiring
ReservationTracker::lock_ in GetReservation(), where an atomic
operation is sufficient.

Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
---
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/bufferpool/reservation-tracker.h
M be/src/util/memory-metrics.cc
3 files changed, 24 insertions(+), 11 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Michael Brown (Code Review)]

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

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1: Code-Review+2

I feel comfortable giving +2 here alongside Bikram's +1: I had independently 
seen the DCHECK was due to !ovf; this fix seems straightforward. Thanks for 
doing so.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 23:11:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 22:52:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8929 )

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h@571
PS1, Line 571:  DCHECK(!ovf)
> It shouldn't be. I believe that the properties of decimal mod and the way t
I see. Thanks for the clarification!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 22:41:50 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Bump LLVM to 5.0.0

[Tim Armstrong (Code Review)]

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

Change subject: Bump LLVM to 5.0.0
..


Patch Set 1: Code-Review+2

Looks good, I think we can wait until the Impala change is ready to go in 
before merging though


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9b7b97cb135d202eaa9a0bae03e722a2505b712
Gerrit-Change-Number: 8932
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 22:37:17 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump LLVM to 5.0.0

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8932


Change subject: Bump LLVM to 5.0.0
..

Bump LLVM to 5.0.0

Testing:
Was able to build on all supported OSes. Also got Impala to build and
pass tests.

Change-Id: Ib9b7b97cb135d202eaa9a0bae03e722a2505b712
---
M buildall.sh
1 file changed, 4 insertions(+), 2 deletions(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9b7b97cb135d202eaa9a0bae03e722a2505b712
Gerrit-Change-Number: 8932
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Michael Ho (Code Review)]

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

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 19:

Build failed due to failed test:

 21:45:15 ] FAIL 
custom_cluster/test_admission_controller.py::TestAdmissionController::()::test_set_request_pool
21:45:15 ] === FAILURES 
===
21:45:15 ]  TestAdmissionController.test_set_request_pool 
_
21:45:15 ] hs2/hs2_test_suite.py:48: in add_session
21:45:15 ] fn(self)
21:45:15 ] custom_cluster/test_admission_controller.py:274: in 
test_set_request_pool
21:45:15 ] ['MEM_LIMIT=2', 'REQUEST_POOL=root.queueB'])
21:45:15 ] custom_cluster/test_admission_controller.py:204: in 
__check_query_options
21:45:15 ] assert len(confs) == len(expected_query_options)
21:45:15 ] E   assert 3 == 2
21:45:15 ] E+  where 3 = len(['MEM_LIMIT=2', 
'REQUEST_POOL=root.queueB', 'IDLE_SESSION_TIMEOUT=0'])
21:45:15 ] E+  and   2 = len(['MEM_LIMIT=2', 
'REQUEST_POOL=root.queueB'])
21:45:15 ]  Captured stdout setup 
-


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 19
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 21:53:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81
Gerrit-Change-Number: 8854
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 21:49:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 19: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 19
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 21:45:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6318: Adjustment for hanging query cancellation test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6318: Adjustment for hanging query cancellation test
..

IMPALA-6318: Adjustment for hanging query cancellation test

Apparently test_query_cancellation_during_fetch hangs occasionally
in Jenkins builds. The Impala debug page shows the query being
cancelled, however, on the host the ImpalaShell process related to
that query is still running.

Since I had no luck in reproducing the issue locally I only have a
theory what might be going on here: The query is cancelled
successfully on Impala backend and when the test tries to get the
stdout and stderr from the ImpalaShell it gets stuck. It might be
the case that ImpalaShell process fetching the query results holds
the stdout. According to the documentation of subprocess.communicate()
it may cause issues to fetch data when the data size is large or
unlimited, that we can consider to be the case here.
As a workaround there is a new optional parameter to
util.ImpalaShell to omit the stdout because this test wouldn't use
it anyway and we get rid of fetching the large result from
ImpalaShell.

Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Reviewed-on: http://gerrit.cloudera.org:8080/8852
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M tests/shell/test_shell_commandline.py
M tests/shell/util.py
2 files changed, 13 insertions(+), 9 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Gerrit-Change-Number: 8852
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6318: Adjustment for hanging query cancellation test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6318: Adjustment for hanging query cancellation test
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Gerrit-Change-Number: 8852
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 20:32:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h@571
PS1, Line 571:  DCHECK(!ovf)
> is overflow here a valid case? if yes, should we return it in '*overflow' i
It shouldn't be. I believe that the properties of decimal mod and the way the 
planner chooses the output precision/scale guarantee that the result should 
always fit in the output type



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 20:27:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

[Adam Holley (Code Review)]

Adam Holley has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" 
command
..

IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command

Updated the sql parser and added code to properly update the
metadatad.

Testing:
Added parser tests and unit tests for alter statements including
partition options.

Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
---
M common/thrift/JniCatalog.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M tests/query_test/test_delimited_text.py
6 files changed, 324 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 3
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 

[Impala-ASF-CR] IMPALA-6355: fix overflow DCHECK in decimal mod

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8929 )

Change subject: IMPALA-6355: fix overflow DCHECK in decimal mod
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8929/1/be/src/runtime/decimal-value.inline.h@571
PS1, Line 571:  DCHECK(!ovf)
is overflow here a valid case? if yes, should we return it in '*overflow' 
instead of DCHECKing ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Gerrit-Change-Number: 8929
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Wed, 03 Jan 2018 20:11:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation

[Tim Armstrong (Code Review)]

Hello Tianyi Wang, Dan Hecht,

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

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

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

Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
..

IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation

In preparation for switching the I/O mgr to the buffer pool, this
removes and cleans up a lot of code so that the switchover patch starts
from a cleaner slate.

* Remove the free buffer cache (which will be replaced by buffer pool's
  own caching).
* Make memory limit exceeded error checking synchronous (in anticipation
  of having to propagate buffer pool errors synchronously).
* Simplify error propagation - remove the (ineffectual) code that
  enqueued BufferDescriptors containing error statuses.
* Document locking scheme better in a few places, make it part of the
  function signature when it seemed reasonable.
* Move ReturnBuffer() to ScanRange, because it is intrinsically
  connected with the lifecycle of a scan range.
* Separate external ReturnBuffer() and internal CleanUpBuffer()
  interfaces - previously callers of ReturnBuffer() were fudging
  the num_buffers_in_reader accounting to make the external interface work.
* Eliminate redundant state in ScanRange: 'eosr_returned_' and
  'is_cancelled_'.
* Clarify the logic around calling Close() for the last
  BufferDescriptor.
  -> There appeared to be an implicit assumption that buffers would be
 freed in the order they were returned from the scan range, so that
 the "eos" buffer was returned last. Instead just count the number
 of outstanding buffers to detect the last one.
  -> Touching the is_cancelled_ field without holding a lock was hard to
 reason about - violated locking rules and it was unclear that it
 was race-free.
* Remove DiskIoMgr::Read() to simplify interface. It is trivial to
  inline at the callsites.

This will probably regress performance somewhat because of the cache
removal, so my plan is to merge it around the same time as switching
the I/O mgr to allocate from the buffer pool. I'm keeping the patches
separate to make reviewing easier.

Testing:
* Ran exhaustive tests
* Ran the disk-io-mgr-stress-test overnight

Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/scanner-context.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/io/disk-io-mgr-stress.cc
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/request-context.cc
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/test-env.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
19 files changed, 575 insertions(+), 904 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1
Gerrit-Change-Number: 8414
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..


Patch Set 1:

Ping?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 19:15:39 +
Gerrit-HasComments: No

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

[Michael Ho (Code Review)]

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

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


Patch Set 1:

The backport for KUDU-2228 is merged now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 03 Jan 2018 18:33:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8758/10/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/8758/10/be/src/runtime/fragment-instance-state.cc@222
PS10, Line 222: RETURN_IF_ERROR(codegen->FinalizeModule());
> We need to undo this change, we deliberately had the the heavyweight codege
Would you mind elaborating on the point about "deliberately had the heavyweight 
codegen in Open()" ?

Now that I look at it again, I realize moving it here could block callers 
waiting for prepared_promise_ to be set to True, which includes cancellation 
requests. Lars, sorry for missing that.

That said, is there any reason why we cannot move the relatively lightweight 
codegen work in Prepare() to Open() ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 18:22:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 19:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 19
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 18:09:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6128: Add support for AES-CTR encryption when spilling to disk CFB mode is a stream cipher and is secure when used with a different nonce/IV for every message. However it can be

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6128: Add support for AES-CTR encryption when spilling 
to disk CFB mode is a stream cipher and is secure when used with a different 
nonce/IV for every message. However it can be a performance bottleneck. CTR 
mode is also stream cipher and is secure
..


Patch Set 3:

(after Bikram has also taken another look to make sure his comments were 
addressed)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff
Gerrit-Change-Number: 8861
Gerrit-PatchSet: 3
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Comment-Date: Wed, 03 Jan 2018 17:07:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6128: Add support for AES-CTR encryption when spilling to disk CFB mode is a stream cipher and is secure when used with a different nonce/IV for every message. However it can be

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6128: Add support for AES-CTR encryption when spilling 
to disk CFB mode is a stream cipher and is secure when used with a different 
nonce/IV for every message. However it can be a performance bottleneck. CTR 
mode is also stream cipher and is secure
..


Patch Set 3:

(4 comments)

Looks great, thanks for the contribution. I just had a few minor requests then 
I can start the  merge.

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

http://gerrit.cloudera.org:8080/#/c/8861/3/be/src/util/openssl-util-test.cc@51
PS3, Line 51: TEST_F(OpenSSLUtilTest, Encryption) {
We should also test both CTR and CFB in this test (this is the basic test that 
encryption works).


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

http://gerrit.cloudera.org:8080/#/c/8861/3/be/src/util/openssl-util.h@93
PS3, Line 93: setCipherMode
nit: initial letter of functions names is capitalised, i.e. SetCipherMode


http://gerrit.cloudera.org:8080/#/c/8861/3/be/src/util/openssl-util.h@109
PS3, Line 109: getCipher
nit: GetCipher()


http://gerrit.cloudera.org:8080/#/c/8861/3/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

http://gerrit.cloudera.org:8080/#/c/8861/3/be/src/util/openssl-util.cc@152
PS3, Line 152:   if (mode_ == AES_256_CTR && EVP_aes_256_ctr) {
nit: if fits on one line, i.e.

  if (mode_ == AES_256_CTR && EVP_aes_256_ctr) return EVP_aes_256_ctr();



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff
Gerrit-Change-Number: 8861
Gerrit-PatchSet: 3
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Comment-Date: Wed, 03 Jan 2018 17:07:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3651: Adds murmur hash() built-in function

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3651: Adds murmur_hash() built-in function
..


Patch Set 2:

(3 comments)

Looks good, just a few minor requests.

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

http://gerrit.cloudera.org:8080/#/c/8893/2//COMMIT_MSG@13
PS2, Line 13: Add unit tests for primitive types
Can you add a test query to exprs.test, just to make sure it works end-to-end?


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

http://gerrit.cloudera.org:8080/#/c/8893/2/be/src/exprs/expr-test.cc@4564
PS2, Line 4564:   // Test murmur_hash
nit: comment is not really needed. Maybe it would actually be best to make the 
new tests a separate TEST_F function?


http://gerrit.cloudera.org:8080/#/c/8893/2/be/src/exprs/expr-test.cc@4571
PS2, Line 4571: expected = HashUtil::MurmurHash2_64(s.data(), s.size(), 
HashUtil::MURMUR_DEFAULT_SEED);
In the cases where we're hashing a constant, can you add a test that compares 
to a constant? Just so we detect if MurmurHash2_64 accidentally changes 
behaviour?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14d56ffb8fab256f3f66a2669271fd4b3c50cc29
Gerrit-Change-Number: 8893
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 16:58:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6318: Adjustment for hanging query cancellation test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6318: Adjustment for hanging query cancellation test
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Gerrit-Change-Number: 8852
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 16:53:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6318: Adjustment for hanging query cancellation test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6318: Adjustment for hanging query cancellation test
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Gerrit-Change-Number: 8852
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 16:53:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6318: Adjustment for hanging query cancellation test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6318: Adjustment for hanging query cancellation test
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I082c83b91b6d0c527de92c7992f0dc9d1b290433
Gerrit-Change-Number: 8852
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 16:52:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6296: Avoid crash caused by DCHECK in Codegen in debug mode

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6296: Avoid crash caused by DCHECK in Codegen in debug 
mode
..


Patch Set 5: Code-Review+1

Thank Mansi. I'll let Thomas do the +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e2b1efb325100d01d398e68e789d87b877167e
Gerrit-Change-Number: 8923
Gerrit-PatchSet: 5
Gerrit-Owner: Manaswini Maharana 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 03 Jan 2018 16:50:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6296: Avoid crash caused by DCHECK in Codegen in debug mode

[Manaswini Maharana (Code Review)]

Manaswini Maharana has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/8923 )

Change subject: IMPALA-6296: Avoid crash caused by DCHECK in Codegen in debug 
mode
..

IMPALA-6296: Avoid crash caused by DCHECK in Codegen in debug mode

Currently, when debug mode is enabled, any query using codegen can result
in an Impala daemon crash as it hits a DCHECK.

This patch ensures the DCHECK is hit only when specific condition is met to
avoid the crash. That condition here is to DCHECK only when 'emit_perf_map_'
evaluates to True ensuring 'perf_map_lock_' is not empty when asserted.

Change-Id: I93e2b1efb325100d01d398e68e789d87b877167e
---
M be/src/codegen/codegen-symbol-emitter.cc
1 file changed, 6 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e2b1efb325100d01d398e68e789d87b877167e
Gerrit-Change-Number: 8923
Gerrit-PatchSet: 5
Gerrit-Owner: Manaswini Maharana 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] Added the "SET ROW FORMAT" option to the "ALTER TABLE" command.

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8928 )

Change subject: Added the "SET ROW FORMAT" option to the "ALTER TABLE" command.
..


Patch Set 1:

(11 comments)

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

http://gerrit.cloudera.org:8080/#/c/8928/1//COMMIT_MSG@7
PS1, Line 7: Added the "SET ROW FORMAT" option to the "ALTER TABLE" command.
Adds "IMPALA-4323: " at the start of the line.
Adds "Testing:" description.


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup@1003
PS1, Line 1003:   | KW_ALTER KW_TABLE table_name:table 
opt_partition_set:partition KW_SET
Add some unit tests(white & black) into ParserTest.java. See
https://github.com/cloudera/Impala/blob/cdh5-trunk/fe/src/test/java/org/apache/impala/analysis/ParserTest.java#L2317


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@35
PS1, Line 35:
nit: use space instead of tab


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2231
PS1, Line 2231:   private boolean alterTableSetFileFormat(Table tbl,
I guess you might use the code in this function. I think there are some 
redundant code. Would you please check there is any room to refactor the both 
code? I am worry about the following case. Let's assume:
1. Function foo has a bug and a patch should be applied to the function.
2. Function boo is spawned from the function foo.
3. Unfortunately a developer isn't aware of function boo and the bug can exist 
in boo.


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2269
PS1, Line 2269:   private boolean alterTableSetRowFormat(Table tbl, 
List partitionSet,
nit: all lines should be limited to 90 characters in width


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2283
PS1, Line 2283: sd.getSerdeInfo().putToParameters("field.delim", 
rowFormat.getFieldDelimiter());
nit: exceeds the limit


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2293
PS1, Line 2293: if (tbl instanceof HdfsTable) ((HdfsTable) 
tbl).addDefaultPartition(msTbl.getSd());
nit: exceeds the limit


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2305
PS1, Line 2305: 
partition.getSerdeInfo().putToParameters("field.delim", 
rowFormat.getFieldDelimiter());
nit: exceeds the limit


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2308
PS1, Line 2308: 
partition.getSerdeInfo().putToParameters("escape.delim", 
rowFormat.getEscapeChar());
nit: exceeds the limit


http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2311
PS1, Line 2311: 
partition.getSerdeInfo().putToParameters("line.delim", 
rowFormat.getLineDelimiter());
nit: exceeds the limit


http://gerrit.cloudera.org:8080/#/c/8928/1/tests/query_test/test_delimited_text.py
File tests/query_test/test_delimited_text.py:

http://gerrit.cloudera.org:8080/#/c/8928/1/tests/query_test/test_delimited_text.py@164
PS1, Line 164:   def test_delimited_text_partitioned(self, vector, 
unique_database):
This test is very similar to test_delimited_text_alter. Do you refactor the 
both code?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154
Gerrit-Change-Number: 8928
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Wed, 03 Jan 2018 13:56:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable

[Impala Public Jenkins (Code Review)]

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

Change subject: KUDU-2228: Make Messenger options configurable
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba
Gerrit-Change-Number: 8878
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 03 Jan 2018 11:45:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable

[Impala Public Jenkins (Code Review)]

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

Change subject: KUDU-2228: Make Messenger options configurable
..

KUDU-2228: Make Messenger options configurable

Currently, the RPC layer accesses many gflags directly to take
certain decisions, eg. whether to turn on encryption,
authentication, etc.

Since the RPC layer is to be used more like a library, these should
be configurable options that are passed to the Messenger
(which is the API endpoint for the application using the RPC layer),
instead of the RPC layer itself directly accessing these flags.

This patch converts the following flags to Messenger options and moves
the flag definitions to server_base.cc which is the "application" in
Kudu that uses the Messenger:

FLAGS_rpc_default_keepalive_time_ms
FLAGS_rpc_negotiation_timeout_ms
FLAGS_rpc_authentication
FLAGS_rpc_encryption
FLAGS_rpc_tls_ciphers
FLAGS_rpc_tls_min_protocol
FLAGS_rpc_certificate_file
FLAGS_rpc_private_key_file
FLAGS_rpc_ca_certificate_file
FLAGS_rpc_private_key_password_cmd
FLAGS_keytab_file

Most of the remaining flags are test or benchmark related flags. There
may be a few more flags that can be moved out and converted to options,
but we can leave that as future work if we decide to move them.

In addition to the cherry-pick above, this change also updates Impala
code to pass the key_tab file to InitKerberosForServer() which was changed
by this Kudu patch.

Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba
Reviewed-on: http://gerrit.cloudera.org:8080/8789
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
Reviewed-on: http://gerrit.cloudera.org:8080/8878
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/client_negotiation.cc
M be/src/kudu/rpc/client_negotiation.h
M be/src/kudu/rpc/messenger.cc
M be/src/kudu/rpc/messenger.h
M be/src/kudu/rpc/negotiation.cc
M be/src/kudu/rpc/negotiation.h
M be/src/kudu/rpc/reactor.cc
M be/src/kudu/rpc/rpc-test-base.h
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/sasl_common.cc
M be/src/kudu/rpc/sasl_common.h
M be/src/kudu/rpc/server_negotiation.cc
M be/src/kudu/rpc/server_negotiation.h
M be/src/kudu/security/CMakeLists.txt
M be/src/kudu/security/init.cc
M be/src/kudu/security/init.h
A be/src/kudu/security/security_flags.cc
A be/src/kudu/security/security_flags.h
M be/src/kudu/security/test/mini_kdc-test.cc
M be/src/kudu/security/tls_context.cc
M be/src/kudu/security/tls_context.h
M be/src/kudu/util/flags.cc
M be/src/kudu/util/flags.h
M be/src/rpc/authentication.cc
24 files changed, 381 insertions(+), 283 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba
Gerrit-Change-Number: 8878
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Zoltan Borok-Nagy (Code Review)]

Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila 
Jeges, Tim Armstrong, Csaba Ringhofer, Dan Hecht,

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

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

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

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.

idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

After this commit, the session timeout can be overridden to
any value, i.e. the command line flag idle_session_timeout
doesn't limit this option anymore.

I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout. I also
extended the test_session_expiration and test_set_and_unset
test suites.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
M tests/custom_cluster/test_session_expiration.py
M tests/custom_cluster/test_set_and_unset.py
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_hs2.py
14 files changed, 381 insertions(+), 59 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 19
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

[Zoltan Borok-Nagy (Code Review)]

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

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 18:

(3 comments)

Thanks!

http://gerrit.cloudera.org:8080/#/c/8490/17/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/17/be/src/service/impala-server.cc@1837
PS17, Line 1837: ile (true) {
> I suppose the reason why we can get rid of the notification here is that be
Right, that's the logic behind it.


http://gerrit.cloudera.org:8080/#/c/8490/18/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/18/be/src/service/impala-server.cc@1829
PS18, Line 1829:
> nit: indent off by 2
Oops, done.


http://gerrit.cloudera.org:8080/#/c/8490/18/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

http://gerrit.cloudera.org:8080/#/c/8490/18/common/thrift/ImpalaInternalService.thrift@292
PS18, Line 292: are never expired.
> nit: never expire.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 18
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 10:20:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable

[Impala Public Jenkins (Code Review)]

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

Change subject: KUDU-2228: Make Messenger options configurable
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba
Gerrit-Change-Number: 8878
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 03 Jan 2018 08:09:53 +
Gerrit-HasComments: No