[Impala-ASF-CR] IMPALA-3866 Improve error reporting for scratch write errors

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9420 )

Change subject: IMPALA-3866 Improve error reporting for scratch write errors
..


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-test.cc
File be/src/runtime/io/disk-io-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-test.cc@191
PS5, Line 191: const string& function_name, int err_no,
> nit: It would be nice to make formal parameter names consistent across func
Done


http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-test.cc@350
PS5, Line 350: const string& function_name, int err_no,
> same as above
Done


http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-test.cc@378
PS5, Line 378: file_nam
> nit: 'file_name' to be consistent with the formal parameter name in Execute
Done


http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-with-fault-injection.h
File be/src/runtime/io/disk-io-mgr-with-fault-injection.h:

http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-with-fault-injection.h@56
PS5, Line 56: eFault
> nit: Maybe this should be called 'err_no_' for consistency?
Hmm. err_no_ just doesn't look that pretty for for me. May I leave this as it 
is?


http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-with-fault-injection.cc
File be/src/runtime/io/disk-io-mgr-with-fault-injection.cc:

http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-with-fault-injection.cc@38
PS5, Line 38: ction("op
> Another approach would be to abstract away the open() syscall itself, inste
Done


http://gerrit.cloudera.org:8080/#/c/9420/5/be/src/runtime/io/disk-io-mgr-with-fault-injection.cc@38
PS5, Line 38: ugFaultInjection("open")) { return -1; }
> Not sure about this, but maybe the order of operands should be reversed (he
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7
Gerrit-Change-Number: 9420
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 07:55:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 08 Mar 2018 07:39:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/9537 )

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..

IMPALA-6615: CTE query in Impala shell does not show query web links

This patch is to show query submitted and query progress web links in
Impala shell for CTE queries.

Testing:
- Ran end-to-end shell tests

Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
---
M shell/impala_shell.py
M tests/shell/test_shell_commandline.py
2 files changed, 8 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..

IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

Quick fix of Parquet write path until the Parquet community
agrees on the ordering of floating point numbers.

The behavior follows the way fmax()/fmin() works, ie. Impala
will only write NaN into the stats when all the values are NaNs.
This behavior is aligned with the quick fix of Parquet-CPP.

Added e2e tests as well.

Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Reviewed-on: http://gerrit.cloudera.org:8080/9381
Reviewed-by: Lars Volker 
Tested-by: Impala Public Jenkins
---
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
3 files changed, 90 insertions(+), 17 deletions(-)

Approvals:
  Lars Volker: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 07:34:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py@1131
PS1, Line 1131: return self._execute_stmt(query, is_dml=is_dml, 
print_web_link=is_dml)
> The WITH clause is only allowed for query statements like SELECT and UNION
Also we want to print weblinks for SELECT and INSERT queries alike.

For SELECT/INSERT without a WITH clause we print the weblinks today.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 07:27:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[David Knupp (Code Review)]

David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9537 )

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py@1131
PS1, Line 1131: return self._execute_stmt(query, is_dml=is_dml, 
print_web_link=is_dml)
> I guess my question is: why are we not passing print_web_links=True here? W
My guess is that not every query containing a WITH clause is DML, and if not, 
then print_web_link should be False. In other words, we either want:

  self._execute_stmt(query, True, True)

or

  self._execute_stmt(query, False, False)

but never

  self._execute_stmt(query, False, True)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 07:20:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 06:57:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..

IMPALA-6617: Improve diagnostics for debugging.

The root cause of IMPALA-6617 is elusive.
This patch enhances the exception message to help
identify the root cause when we hit IMPALA-6617 again.

Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Reviewed-on: http://gerrit.cloudera.org:8080/9545
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/service/FeSupport.java
1 file changed, 4 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

[Donghui Xu (Code Review)]

Donghui Xu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9506 )

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..


Patch Set 4:

Hi Bharath, Please review this code. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 4
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 06:41:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 4:

Rerunning - failed at waiting for HDFS replication.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 06:18:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[Adam Holley (Code Review)]

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

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 05:44:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show query web links

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9537 )

Change subject: IMPALA-6615: CTE query in Impala shell does not show query web 
links
..

IMPALA-6615: CTE query in Impala shell does not show query web links

This patch is to show query submitted and query progress web links in
Impala shell for CTE queries.

Testing:
- Ran end-to-end shell tests

Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
---
M shell/impala_shell.py
M tests/shell/test_shell_commandline.py
2 files changed, 8 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] IMPALA-6240: [DOCS] Document PARQUET ARRAY RESOLUTION query option

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6240: [DOCS] Document PARQUET_ARRAY_RESOLUTION query 
option
..


Patch Set 2:

(13 comments)

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

http://gerrit.cloudera.org:8080/#/c/9534/2//COMMIT_MSG@10
PS2, Line 10: Cherry-picks: not for 2.x
move above Change-Id


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml
File docs/topics/impala_parquet_array_resolution.xml:

http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@21
PS2, Line 21: 
Wrong refs?


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@47
PS2, Line 47:   order of the indexed-based resolution for nested arrays in 
Parquet.
I don't think "order" is correct here. "depth" would be more appropriate, but 
probably too confusing

suggestion:

... controls the behavior of the index-based field resolution for nested arrays 
in Parquet.


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@48
PS2, Line 48: 
Mention the relevant query option for name vs. index based resolution:

PARQUET_FALLBACK_SCHEMA_RESOLUTION=position


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@50
PS2, Line 50:  In Parquet, you can represent an array using a 2-level or 
3-level
We should clearly state that the modern, standard representation is 3-level. 
The legacy 2-level scheme is supported for compatibility with older Parquet 
files.


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@51
PS2, Line 51:   representation. However, there is no metadata within 
Parquet files to
However, there is no reliable metadata ...


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@81
PS2, Line 81:   All of the above options resolves arrays encoded with a 
single level.
resolve


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@85
PS2, Line 85:   A failure to resolve a schema path with a given array 
resolution policy
A failure to resolve a column/field reference in a query with a given array 
resolution ...


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@87
PS2, Line 87:   A mismatch might be treated like a missing field, and it is 
not possible
... like a missing column (returns NULL values), and it is not possible ...


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@89
PS2, Line 89:   field' cases.
'legitimately missing column'


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@93
PS2, Line 93:   The name-based policy generally does not have the problem 
of ambiguous
Mention how the name based resolution is set:
PARQUET_FALLBACK_SCHEMA_RESOLUTION=name


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@116
PS2, Line 116: SingleFieldGroupInList {
Maybe you can give this a more documenty-name such as

ParquetSchemaExampleA


http://gerrit.cloudera.org:8080/#/c/9534/2/docs/topics/impala_parquet_array_resolution.xml@165
PS2, Line 165: ThriftPrimitiveInList {
ParquetSchemaExampleA? The "Thrift" stuff in the name might confuse readers



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12696b609609ea16c05d8b7e84b2bae0be6d6cb5
Gerrit-Change-Number: 9534
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Thu, 08 Mar 2018 05:19:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show web links

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6615: CTE query in Impala shell does not show web links
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9537/1/shell/impala_shell.py@1131
PS1, Line 1131: return self._execute_stmt(query, is_dml=is_dml, 
print_web_link=is_dml)
We should also print weblinks for DML, since that's what we do without a WITH 
clause.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Thu, 08 Mar 2018 05:14:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..

IMPALA-6592: add test for invalid parquet codecs

IMPALA-6592 revealed a gap in test coverage for files with
invalid/unsupported Parquet codecs. This adds a test that reproduces the
bug that was present in my IMPALA-4835 patch. master is unaffected by
this bug.

I also hid the conversion tables and made the conversion go through
functions that validate the enum values, to make it easier to track down
problems like this in the future.

Testing:
Ran exhaustive tests.

Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Reviewed-on: http://gerrit.cloudera.org:8080/9500
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exec/CMakeLists.txt
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/parquet-column-readers.h
A be/src/exec/parquet-common.cc
M be/src/exec/parquet-common.h
M be/src/util/parquet-reader.cc
M testdata/data/README
A testdata/data/bad_codec.parquet
A testdata/workloads/functional-query/queries/QueryTest/parquet-bad-codec.test
M tests/query_test/test_scanners.py
10 files changed, 136 insertions(+), 46 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:48:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6615: CTE query in Impala shell does not show web links

[Adam Holley (Code Review)]

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

Change subject: IMPALA-6615: CTE query in Impala shell does not show web links
..


Patch Set 1:

Can you add a test to validate?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3352406e3b048be395a20405c8e6b911e663164
Gerrit-Change-Number: 9537
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:43:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 4: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:33:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6609: Fix ownership of class members in 
KrpcDataStreamRecvr
..


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.h
File be/src/runtime/krpc-data-stream-recvr.h:

http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.h@163
PS1, Line 163:   /// Row schema.
> Add "Not owned."
Done


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.h@182
PS1, Line 182: client_
> Not your change, but I think we should we rename this to buffer_pool_client
Renamed to buffer_pool_client_.


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.h@201
PS1, Line 201: own most of the
 :   /// counters below
> if the only exception is the one you marked "Not owned", how about saying "
Done


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.h@201
PS1, Line 201: own most of the
 :   /// counters below
> Add "These profiles are guaranteed to live as long as their parent 'profile
Done


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc@a636
PS1, Line 636:
> Maybe I'm missing something, but I'm not able to understand why this was re
TakeOverEarlySender() may access 'mgr_'.

I was thinking that there may be a race between Close() and 
TakeOverEarlySender(). However, the race should be impossible given both 
TakeOverEarlySenders() and Close() are supposed to be called from the fragment 
execution thread context.

Reverted this line in PS2. Added a DCHECK() to validate the assumption.


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc@411
PS1, Line 411: recvr_->deferred_rpc_tracker()
> We access the deferred_rpc_tracker() here too. Should we check for cancella
Another subtle point which warrants a comment. See line 399 for the DCHECK. The 
contract is that the queue 'deferred_rpcs_' is drained during cancellation and 
no thread should add to 'deferred_rpcs_' after Cancel() is called. So, line 398 
implicitly handles the case of cancellation. Note the DCHECK() at line 399. 
Comments needed there.


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc@451
PS1, Line 451: Only update the 'deferred_rpc_tracker_' if the queue is still 
open.
> I think a "why" rather than "what" would be more useful here. Something lik
It's true that a queue may be cancelled but not yet closed if a fragment 
instance is cancelled but not yet closed. We want to avoid consuming from 
'deferred_rpc_trackers_' as all deferred RPCs are drained in Cancel() so 
deferred_rpc_trackers_'s consumption should go to zero after Cancel() has been 
called on all sender queues.

There is a window in KrpcDataStreamMgr::CreateRecvr() between FindRecvr() 
returns and when TakeOverEarlySender is called. In that window, the fragment 
instance can be cancelled by the coordinator. That's how a thread may end up 
calling TakeOverEarlySender() after it has been cancelled. This receiver 
shouldn't be closed at this point as this function is called in the context of 
the fragment execution thread and KrpcDataStreamReceiver::Close() is also 
supposed to be called from fragment execution thread only.

Added some explanation and DCHECK for it.


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc@568
PS1, Line 568: Note that the parent profile doesn't hold any ownership to these
 :   // profiles.
> I think this will be hard to decifer, is there a more direct thing we can s
This is trying to point out that 'profile_' won't actually free these two 
profiles so they are safe to access even after 'profile_' is deleted. Added the 
comments you suggested.


http://gerrit.cloudera.org:8080/#/c/9527/1/be/src/runtime/krpc-data-stream-recvr.cc@644
PS1, Line 644: deferred_rpc_tracker_->Close();
> This seems a little brittle, although it would work for us. In the header,
Yes, I believe a more clear explanation about the contract is warranted. In 
essence, it's assuming that all threads which try to consume from / insert into 
sender queues will hold the sender queue's lock and check for 'is_cancelled_' 
flag which acts as a surrogate on whether the queue is still open for business. 
If 'is_cancelled_' is true, it may mean Close() has been called on it already. 
So past line 642 above, no threads should really be consuming from 
deferred_rpc_tracker_ so the main thread here can safely close it without 
holding the lock.



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

Gerrit-Project: 

[Impala-ASF-CR] IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-6609: Fix ownership of class members in 
KrpcDataStreamRecvr
..

IMPALA-6609: Fix ownership of class members in KrpcDataStreamRecvr

A KrpcDataStreamRecvr is co-owned by the singleton KrpcDataStreamMgr
and an exchange node. It's possible that some threads (e.g. RPC service
threads) may still retain reference to the KrpcDataStreamRecvr after its
owning exchange node has been closed. This is problematic as some members
in the receiver (e.g. sender/receiver profiles) are actually owned by the
exchange node so accessing them after the exchange node is closed and
possibly deleted may lead to user-after-free.

This patch changes the ownership of some members in KrpcDataStreamRecvr
to the receiver. In particular, the profiles are now owned by the receiver
and various stat counters and timers are in turn owned by these profiles.
This prevents the use-after-free problem described above. This patch also
moves the access to deferred_rpc_tracker_ in TakeOverEarlySenders() to be
under the sender queue's 'lock_' to prevent another instance of IMPALA-6554.

Testing done: core debug build.

Change-Id: I3378496e2201b16c662b9daa634333480705c61a
---
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/query-state.cc
M be/src/util/thread.cc
M be/src/util/thread.h
7 files changed, 105 insertions(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a
Gerrit-Change-Number: 9527
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6573: Create consistent response on column access failures

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9509 )

Change subject: IMPALA-6573: Create consistent response on column access 
failures
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9509/2/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizeable.java:

http://gerrit.cloudera.org:8080/#/c/9509/2/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java@56
PS2, Line 56: }
> I wonder what was changed here. If it's unintentional, can you revert the c
There seems to be something weird with Gerrit. I can't see this change when I 
pulled your review.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
Gerrit-Change-Number: 9509
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:05:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 03:54:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 03:54:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 03:53:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6614: ClientRequestState should use HS2 TOperationState

[Bikramjeet Vig (Code Review)]

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

Change subject: IMPALA-6614: ClientRequestState should use HS2 TOperationState
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9501/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/9501/3/be/src/service/client-request-state.cc@a880
PS3, Line 880:
should we check for is_cancelled_ instead to be more explicit and in sync with 
the method comments


http://gerrit.cloudera.org:8080/#/c/9501/3/be/src/service/client-request-state.cc@a884
PS3, Line 884:
maybe mark a cancelled event even if it was cancelled by the user explicitly.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Gerrit-Change-Number: 9501
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 08 Mar 2018 03:17:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:54:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9484 )

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:54:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Alex Behm (Code Review)]

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

Change subject: Fix test dimensions in test_errorlog.py
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:52:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Alex Behm (Code Review)]

Hello Lars Volker,

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

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

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

Change subject: Fix test dimensions in test_errorlog.py
..

Fix test dimensions in test_errorlog.py

Bug: The test dimensions were set up such that
the test ran for all text-format variants
(including compressed text)

The test is file-format independent and slow (>40s),
so we should only run it once.

Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
---
M tests/query_test/test_errorlog.py
1 file changed, 6 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Alex Behm (Code Review)]

Hello Lars Volker,

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

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

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

Change subject: Fix test dimensions in test_errorlog.py
..

Fix test dimensions in test_errorlog.py

Bug: The test dimensions were set up such that
the test ran for all text-format variants
(including compressed text)

The test is file-format independent and slow (>40s),
so we should only run it once.

Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
---
M tests/query_test/test_errorlog.py
1 file changed, 6 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Alex Behm (Code Review)]

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

Change subject: Fix test dimensions in test_errorlog.py
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9546/1/tests/query_test/test_errorlog.py
File tests/query_test/test_errorlog.py:

http://gerrit.cloudera.org:8080/#/c/9546/1/tests/query_test/test_errorlog.py@40
PS1, Line 40:   create_uncompressed_text_dimension(cls.get_workload()))
> nit: indent 4 spaces
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:51:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 3: Code-Review+2

rebase, +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:49:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:50:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:48:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9545/1//COMMIT_MSG@9
PS1, Line 9: 6671
> typo?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:48:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

Hello Lars Volker,

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

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

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..

IMPALA-6617: Improve diagnostics for debugging.

The root cause of IMPALA-6617 is elusive.
This patch enhances the exception message to help
identify the root cause when we hit IMPALA-6617 again.

Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
---
M fe/src/main/java/org/apache/impala/service/FeSupport.java
1 file changed, 4 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9484 )

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..


Patch Set 2:

Apologies I didn't see those tests.

I updated and verified with:
bin/impala-py.test tests/query_test/test_tpch_queries.py


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:47:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Grant Henke (Code Review)]

Hello Thomas Tauber-Marshall, Dimitris Tsirogiannis, Tim Armstrong, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..

IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

Before Kudu supported DECIMAL columns the TPCDS and TPCH
columns were djusted to use DOUBLE in place of DECIMAL. This
patch undoes that change now that Kudu supports DECIMAL.

Testing:
Ran concurrent_select.py on Kudu for tpc-h and tpc-ds.

Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
---
M testdata/datasets/tpcds/tpcds_kudu_template.sql
M testdata/datasets/tpch/tpch_kudu_template.sql
M testdata/datasets/tpch/tpch_schema_template.sql
D testdata/workloads/tpcds/queries/tpcds-kudu-q19.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q27.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q3.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q34.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q42.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q43.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q46.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q47.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q52.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q53.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q55.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q59.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q6.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q61.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q63.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q65.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q68.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q7.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q73.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q79.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q8.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q88.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q89.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q96.test
D testdata/workloads/tpcds/queries/tpcds-kudu-q98.test
D testdata/workloads/tpch/queries/tpch-kudu-q1.test
D testdata/workloads/tpch/queries/tpch-kudu-q10.test
D testdata/workloads/tpch/queries/tpch-kudu-q11.test
D testdata/workloads/tpch/queries/tpch-kudu-q12.test
D testdata/workloads/tpch/queries/tpch-kudu-q13.test
D testdata/workloads/tpch/queries/tpch-kudu-q14.test
D testdata/workloads/tpch/queries/tpch-kudu-q15.test
D testdata/workloads/tpch/queries/tpch-kudu-q16.test
D testdata/workloads/tpch/queries/tpch-kudu-q17.test
D testdata/workloads/tpch/queries/tpch-kudu-q18.test
D testdata/workloads/tpch/queries/tpch-kudu-q19.test
D testdata/workloads/tpch/queries/tpch-kudu-q2.test
D testdata/workloads/tpch/queries/tpch-kudu-q20.test
D testdata/workloads/tpch/queries/tpch-kudu-q21.test
D testdata/workloads/tpch/queries/tpch-kudu-q22.test
D testdata/workloads/tpch/queries/tpch-kudu-q3.test
D testdata/workloads/tpch/queries/tpch-kudu-q4.test
D testdata/workloads/tpch/queries/tpch-kudu-q5.test
D testdata/workloads/tpch/queries/tpch-kudu-q6.test
D testdata/workloads/tpch/queries/tpch-kudu-q7.test
D testdata/workloads/tpch/queries/tpch-kudu-q8.test
D testdata/workloads/tpch/queries/tpch-kudu-q9.test
M tests/query_test/test_tpch_queries.py
M tests/stress/concurrent_select.py
52 files changed, 108 insertions(+), 22,157 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Lars Volker (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 1: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9545/1//COMMIT_MSG@9
PS1, Line 9: 6671
typo?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:43:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6573: Create consistent response on column access failures

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9509 )

Change subject: IMPALA-6573: Create consistent response on column access 
failures
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9509/2/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizeable.java:

http://gerrit.cloudera.org:8080/#/c/9509/2/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java@56
PS2, Line 56: }
I wonder what was changed here. If it's unintentional, can you revert the 
changes to this file?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
Gerrit-Change-Number: 9509
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:37:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9506 )

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 4
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:30:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

[Donghui Xu (Code Review)]

Donghui Xu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9506 )

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..


Patch Set 4:

(2 comments)

I have modified the commit message according to your opinion.

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

http://gerrit.cloudera.org:8080/#/c/9506/3//COMMIT_MSG@9
PS3, Line 9: The value of LDAP password in Impala shell contains extra line 
break.
> nit: "The value of LDAP password in Impala shell contains extra line break.
Done


http://gerrit.cloudera.org:8080/#/c/9506/3//COMMIT_MSG@11
PS3, Line 11: I fixed the issue by removing extraneous '\n' in the LDAP 
password.
> nit: "I fixed the issue by removing extraneous '\n' in the LDAP password."
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 4
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:28:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6610: Impala shell fetches the value of ldap password cmd incorrectly

[Donghui Xu (Code Review)]

Hello Fredy Wijaya,

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

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

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

Change subject: IMPALA-6610: Impala shell fetches the value of 
ldap_password_cmd incorrectly
..

IMPALA-6610: Impala shell fetches the value of ldap_password_cmd incorrectly

The value of LDAP password in Impala shell contains extra line break.

I fixed the issue by removing extraneous '\n' in the LDAP password.

Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
---
M shell/impala_shell.py
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 4
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Lars Volker (Code Review)]

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

Change subject: Fix test dimensions in test_errorlog.py
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:16:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Lars Volker (Code Review)]

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

Change subject: Fix test dimensions in test_errorlog.py
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9546/1/tests/query_test/test_errorlog.py
File tests/query_test/test_errorlog.py:

http://gerrit.cloudera.org:8080/#/c/9546/1/tests/query_test/test_errorlog.py@40
PS1, Line 40:   create_uncompressed_text_dimension(cls.get_workload()))
nit: indent 4 spaces



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
Gerrit-Change-Number: 9546
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:16:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add fragment instance id/query id to important log messages and make them grep friendly

[Impala Public Jenkins (Code Review)]

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

Change subject: Add fragment_instance_id/query_id to important log messages and 
make them grep friendly
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Gerrit-Change-Number: 9535
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 08 Mar 2018 02:14:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] Add fragment instance id/query id to important log messages and make them grep friendly

[Impala Public Jenkins (Code Review)]

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

Change subject: Add fragment_instance_id/query_id to important log messages and 
make them grep friendly
..

Add fragment_instance_id/query_id to important log messages and make them grep 
friendly

Debugging via going through log messages can get very tricky when
the fragment_instance_id and query_id are missing from certain important
log messages. This patch tries to add it in places where I've had issues
in the past.

This patch also tries to standardize on 'fragment_instance_id' and
'query_id' over all log messages so that it's easy to grep through the
logs while debugging.

There may be more places that I've missed, but we can fix them as we
come across them.

Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Reviewed-on: http://gerrit.cloudera.org:8080/9535
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
6 files changed, 37 insertions(+), 30 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Gerrit-Change-Number: 9535
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..


Patch Set 14:

(17 comments)

It looks like we're getting close.

http://gerrit.cloudera.org:8080/#/c/7009/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/7009/14//COMMIT_MSG@25
PS14, Line 25: in expr-test.cc to be inline with existing tests for CAST() 
function.
I think we still need some end-to-end tests that exercise the new function in 
the context of a query, and probably also one that scans a text table with this 
format. Let me know if I can help with setting those up.


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.h
File be/src/runtime/timestamp-parse-util.h:

http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.h@182
PS14, Line 182: parse
nit: "to be parsed" here and below.


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@217
PS14, Line 217: const char* TimestampParser::ParseDigitToken(const char* str, 
const char* str_end){
nit: needs space


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@229
PS14, Line 229:   while(tok_end < str_end) {
nit: needs space


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@236
PS14, Line 236: bool 
TimestampParser::ParseFormatTokensByStr(DateTimeFormatContext* dt_ctx){
nit: needs space


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@239
PS14, Line 239:   DCHECK(dt_ctx->fmt_len > 0);
DCHECK_GT(dt_ctx->fmt_len, 0);


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@240
PS14, Line 240: size
DCHECK_EQ(dt_ctx->toks.size(), 0);


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@249
PS14, Line 249: dt_ctx->toks.push_back(DateTimeFormatToken(YEAR,str - 
str_begin, tok_end - str,
nit: needs space


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@250
PS14, Line 250: str));
nit: replace tab with spaces


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@263
PS14, Line 263: dt_ctx->toks.push_back(DateTimeFormatToken(MONTH_IN_YEAR, 
str - str_begin, tok_end - str,
nit: > 90 chars


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@283
PS14, Line 283:  dt_ctx->has_date_toks
We can just return true here, right?


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@340
PS14, Line 340: factional
typo: fractional


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@346
PS14, Line 346: if (tok_end - str > 9) {
This is really equivalent to the following, right?

  int num_digits = min(9, tok_end - str);
  dt_ctx->toks.push_back(DateTimeFormatToken(FRACTION, str - str_begin, 
num_digits, str));


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@357
PS14, Line 357: dt_ctx->has_time_toks
this can just be 'true', right?


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@457
PS14, Line 457: NULL
nit: nullptr


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@458
PS14, Line 458:   lazy_ctx.Reset(str, lazy_len);
It's annoying that we have to pay the cost of initializing lazy_ctx when we 
don't use it. It's probably not that bad, but I'd rather that the cost of this 
extra functionality was as close to zero as possible. Can we avoid that by 
restructuring this as:

  if (dt_ctx != nullptr) {
return Parse(str, len, *dt_ctx, d, t);
  } else {
DateTimeFormatContext lazy_ctx;
if (ParseFormatTokensByStr(_ctx)) {
  // Use lazy_ctx
} else {

  *d = boost::gregorian::date();
  *t = boost::posix_time::time_duration(boost::posix_time::not_a_date_time);
  return false;
}
  }


http://gerrit.cloudera.org:8080/#/c/7009/14/be/src/runtime/timestamp-parse-util.cc@460
PS14, Line 460:   dt_ctx = _ctx;
nit: indentation



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-Change-Number: 7009
Gerrit-PatchSet: 14
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:42:29 +

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:05:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9500/3/be/src/exec/parquet-common.h
File be/src/exec/parquet-common.h:

http://gerrit.cloudera.org:8080/#/c/9500/3/be/src/exec/parquet-common.h@39
PS3, Line 39: vlaidate
> validate
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:05:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:05:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Tim Armstrong (Code Review)]

Hello Zoltan Borok-Nagy, Dan Hecht,

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

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

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..

IMPALA-6592: add test for invalid parquet codecs

IMPALA-6592 revealed a gap in test coverage for files with
invalid/unsupported Parquet codecs. This adds a test that reproduces the
bug that was present in my IMPALA-4835 patch. master is unaffected by
this bug.

I also hid the conversion tables and made the conversion go through
functions that validate the enum values, to make it easier to track down
problems like this in the future.

Testing:
Ran exhaustive tests.

Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
---
M be/src/exec/CMakeLists.txt
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/parquet-column-readers.h
A be/src/exec/parquet-common.cc
M be/src/exec/parquet-common.h
M be/src/util/parquet-reader.cc
M testdata/data/README
A testdata/data/bad_codec.parquet
A testdata/workloads/functional-query/queries/QueryTest/parquet-bad-codec.test
M tests/query_test/test_scanners.py
10 files changed, 136 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9500 )

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9500/3/be/src/exec/parquet-common.h
File be/src/exec/parquet-common.h:

http://gerrit.cloudera.org:8080/#/c/9500/3/be/src/exec/parquet-common.h@39
PS3, Line 39: vlaidate
validate



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:01:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:00:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 01:00:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@100
PS4, Line 100: if exit_code == 5:
> I wonder if this would work?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:50:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Vuk Ercegovac (Code Review)]

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-6602: fixes flaky expiration test
..

IMPALA-6602: fixes flaky expiration test

The test_query_expiration test assumes that a metric and
the query state are maintained atomically. Since they're
not, occasionaly flakes (false negatives) occur.

The fix in this patch is to loop until the expected state
is seen. If the expected state is not seen with a given number
of iterations, the test fails. These tests depend on timing so
if this validation takes too long, the test will also fail.
Such looping is used in the two places where its assumed that the
client state and metrics are maintained atomically.

Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
---
M tests/custom_cluster/test_query_expiration.py
1 file changed, 19 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9538 )

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py@176
PS3, Line 176: unable
> nit: long line
Done


http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py@183
PS3, Line 183: has_expected_state
> actually, one more request - can we include the actual state in the asserti
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:46:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@244
PS4, Line 244: print_metrics('connections')
> Please make sure to maintain consistent print_metrics() usage. I think you
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:41:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..

IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit_code 5

exit_code for EE tests when no tests are collected.After this
change return_code will be either 0 if no tests are expected
to be collected (dry-run) and 1 if tests are expected to be
collected but are not collected due to some error

Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
---
M tests/run-tests.py
1 file changed, 65 insertions(+), 18 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 8
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py@176
PS3, Line 176: unable
nit: long line


http://gerrit.cloudera.org:8080/#/c/9538/3/tests/custom_cluster/test_query_expiration.py@183
PS3, Line 183: has_expected_state
actually, one more request - can we include the actual state in the assertion 
message to help with debugging?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:34:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:33:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:33:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Zoltan Borok-Nagy (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 3: Code-Review+1

LGTM, again :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:31:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] Fix test dimensions in test errorlog.py

[Alex Behm (Code Review)]

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


Change subject: Fix test dimensions in test_errorlog.py
..

Fix test dimensions in test_errorlog.py

Bug: The test dimensions were set up such that
the test ran for all text-format variants
(including compressed text)

The test is file-format independent and slow (>40s),
so we should only run it once.

Change-Id: Icac90d308337f2cfb51e7de5bd23d410da073a73
---
M tests/query_test/test_errorlog.py
1 file changed, 6 insertions(+), 5 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Vuk Ercegovac (Code Review)]

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-6602: fixes flaky expiration test
..

IMPALA-6602: fixes flaky expiration test

The test_query_expiration test assumes that a metric and
the query state are maintained atomically. Since they're
not, occasionaly flakes (false negatives) occur.

The fix in this patch is to loop until the expected state
is seen. If the expected state is not seen with a given number
of iterations, the test fails. These tests depend on timing so
if this validation takes too long, the test will also fail.
Such looping is used in the two places where its assumed that the
client state and metrics are maintained atomically.

Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
---
M tests/custom_cluster/test_query_expiration.py
1 file changed, 18 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc
File be/src/exec/parquet-common.cc:

http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc@24
PS2, Line 24: static
> Nit: namespace-scoped const variables are implicitly have internal linkage
Done


http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc@45
PS2, Line 45: sizeof(INTERNAL_TO_PARQUET_TYPES) / 
sizeof(INTERNAL_TO_PARQUET_TYPES[0])
> Nit: maybe you could use std::array instead.
>From what I can tell, it's not currently possible to create a std::array 
>without specifying the size directly. There's a proposal for make_array but it 
>doesn't look like it's standardised. I could hard-code the size, but that 
>overall doesn't seem to result in a simpler solution.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:24:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Tim Armstrong (Code Review)]

Hello Zoltan Borok-Nagy,

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

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

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..

IMPALA-6592: add test for invalid parquet codecs

IMPALA-6592 revealed a gap in test coverage for files with
invalid/unsupported Parquet codecs. This adds a test that reproduces the
bug that was present in my IMPALA-4835 patch. master is unaffected by
this bug.

I also hid the conversion tables and made the conversion go through
functions that validate the enum values, to make it easier to track down
problems like this in the future.

Testing:
Ran exhaustive tests.

Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
---
M be/src/exec/CMakeLists.txt
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/parquet-column-readers.h
A be/src/exec/parquet-common.cc
M be/src/exec/parquet-common.h
M be/src/util/parquet-reader.cc
M testdata/data/README
A testdata/data/bad_codec.parquet
A testdata/workloads/functional-query/queries/QueryTest/parquet-bad-codec.test
M tests/query_test/test_scanners.py
10 files changed, 136 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@80
PS1, Line 80: assert (client.get_state(short_timeout_expire_handle) ==
> my thought here is that we think we wait enough so that we're just looking
Ah yeah, that makes sense.


http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@175
PS1, Line 175:   def __expect_client_state(self, client, handle, 
expected_state, max_tries=10):
> we're doing an rpc for this one so I thought it should be slow enough. sinc
I'm ok with a time bound. I don't think it needs to be too loose, since 
cancellation should happen as soon as one of the cancellation threads is 
scheduled. 10s of milliseconds seems reasonable.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:11:35 +
Gerrit-HasComments: Yes

[native-toolchain-CR] IMPALA-5717: Build ORC C++ lib in toolchain

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5717: Build ORC C++ lib in toolchain
..


Patch Set 5:

I've just been running into some infrastructure issues while building this.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6abd86a5a683f19aa44b47629edf283b938b7e7e
Gerrit-Change-Number: 9274
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:09:40 +
Gerrit-HasComments: No

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

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

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


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8523/11/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/8523/11/be/src/scheduling/scheduler.cc@323
PS11, Line 323:   ScanRangeIter range_iter(entry.second);
> sorry if this was already discussed, but why not just transform the list he
idea was to not make multiple passes. common case (for now) is to not explode. 
the current scheme does the least extra work over what we have now.


http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@228
PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec 
hdfs_file_split_generator_spec
> Right, but what I'm thinking is replace listhttp://gerrit.cloudera.org:8080/8523
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:03:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6617: Improve diagnostics for debugging.
..


Patch Set 1:

The new exception message looks like this:

ERROR: IllegalStateException: Illegal expr eval result. Expr=NULL < 10
TExpBatch=TExprBatch(exprs:[TExpr(nodes:[TExprNode(node_type:FUNCTION_CALL, 
type:TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:BOOLEAN))]), num_children:2, is_constant:true, 
fn:TFunction(name:TFunctionName(db_name:_impala_builtins, function_name:lt), 
binary_type:BUILTIN, arg_types:[TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:INT))]), TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:INT))])], 
ret_type:TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:BOOLEAN))]), has_var_args:false, signature:lt(INT, 
INT), 
scalar_fn:TScalarFunction(symbol:_ZN6impala9Operators16Lt_IntVal_IntValEPN10impala_udf15FunctionContextERKNS1_6IntValES6_),
 is_persistent:true)), TExprNode(node_type:NULL_LITERAL, 
type:TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:INT))]), num_children:0, is_constant:true), 
TExprNode(node_type:INT_LITERAL, type:TColumnType(types:[TTypeNode(type:SCALAR, 
scalar_type:TScalarType(type:INT))]), num_children:0, is_constant:true, 
int_literal:TIntLiteral(value:10))])])
Result=TResultRow(colVals:[TColumnValue()])

I manually confirmed that the message is also dumped to the impalad log.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
Gerrit-Change-Number: 9545
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Thu, 08 Mar 2018 00:02:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6617: Improve diagnostics for debugging.

[Alex Behm (Code Review)]

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


Change subject: IMPALA-6617: Improve diagnostics for debugging.
..

IMPALA-6617: Improve diagnostics for debugging.

The root cause of IMPALA-6671 is elusive.
This patch enhances the exception message to help
identify the root cause when we hit IMPALA-6617 again.

Change-Id: I36e2c1185836262759c809632d69d0126af9bdbf
---
M fe/src/main/java/org/apache/impala/service/FeSupport.java
1 file changed, 4 insertions(+), 1 deletion(-)



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

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

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

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

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


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8523/11/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/8523/11/be/src/scheduling/scheduler.cc@323
PS11, Line 323:   ScanRangeIter range_iter(entry.second);
sorry if this was already discussed, but why not just transform the list here 
all at once and pass the exploded list of ranges into 
ComputeScanRangeAssignment()? Is the intention to optimize memory use?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:58:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6592: add test for invalid parquet codecs

[Zoltan Borok-Nagy (Code Review)]

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

Change subject: IMPALA-6592: add test for invalid parquet codecs
..


Patch Set 2: Code-Review+1

(2 comments)

LGTM

http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc
File be/src/exec/parquet-common.cc:

http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc@24
PS2, Line 24: static
Nit: namespace-scoped const variables are implicitly have internal linkage


http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.cc@45
PS2, Line 45: sizeof(INTERNAL_TO_PARQUET_TYPES) / 
sizeof(INTERNAL_TO_PARQUET_TYPES[0])
Nit: maybe you could use std::array instead.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:56:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6573: Create consistent response on column access failures

[Adam Holley (Code Review)]

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

Change subject: IMPALA-6573: Create consistent response on column access 
failures
..

IMPALA-6573: Create consistent response on column access failures

This fixes a bug where failure messages were inconsistent
depending on whether valid or invalid objects were included in the
statement.

Testing:

Created validation tests for both select and describe.

Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
5 files changed, 59 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
Gerrit-Change-Number: 9509
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] IMPALA-6573: Create consistent response on column access failures

[Adam Holley (Code Review)]

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

Change subject: IMPALA-6573: Create consistent response on column access 
failures
..


Patch Set 1:

(1 comment)

I've made changes to switch to column specific.  It is not reproducible with 
tables since the onAnyTable method is not used anywhere.

http://gerrit.cloudera.org:8080/#/c/9509/1/fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java:

http://gerrit.cloudera.org:8080/#/c/9509/1/fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java@62
PS1, Line 62: if (isLastNodeVisible()) {
> It seems like it should also apply to tables, but I've be unable so far to
I've switched it to be column specific.  Tables are handled differently.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
Gerrit-Change-Number: 9509
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:37:27 +
Gerrit-HasComments: Yes

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

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

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


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@228
PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec 
hdfs_file_split_generator_spec
> we currently put all scan ranges from all partitions into the same list. pa
Right, but what I'm thinking is replace list 
with list where that thing is basically a union:

// Needs better name
struct TSplitConcreteOrGenSpec {
   // Only set for concrete range.
   optional Planner.TScanRangeLocationList;
   // Only set for generated spec
   optional THdfsFileSplitGeneratorSpec
}

After all, once we've traversed in TScanRangeLocationList we're now talking 
about physical locations and such, so that stuff seems like it belongs on the 
"concrete" side of the divide.

Maybe I'm still not seeing what you mean by "least damage" though.  "Least 
damage" in what sense?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:30:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0

[Alex Rodoni (Code Review)]

Hello Greg Rahn, Tianyi Wang, John Russell, Alex Behm,

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

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

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

Change subject: IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0
..

IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0

Change-Id: I09dbaf5e2d22f07a8bca8a601c04e94f3a4b36a0
Cherry-picks: not for 2.x
---
M docs/topics/impala_reserved_words.xml
1 file changed, 226 insertions(+), 58 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09dbaf5e2d22f07a8bca8a601c04e94f3a4b36a0
Gerrit-Change-Number: 9540
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@30
PS4, Line 30: from _pytest.runner import runtestprotocol
> Again...handled in the last patch
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:21:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle
> Agreed. And also, please explain testing done.
It has been handled in the latest patch


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@30
PS4, Line 30: from _pytest.runner import runtestprotocol
> Is this used?
Again...handled in the last patch


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
> Please use new-style classes, and inherit from object.
Done.

I would prefer to keep the name as TestStatistics , so  any functions related 
to pytest data can be added here


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
> I see what you are saying. But the other classes (Class Testexecutor) in ru
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@67
PS4, Line 67:   def __init__(self):
:   self.tests_collected = []
:   self.tests_executed = []
> 2 spaces, please.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@71
PS4, Line 71: # items represents the list of collected test items
> Agreed.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@72
PS4, Line 72:   def pytest_collection_modifyitems(self, items):
> For these special hooks I think it's important to note that the name is sig
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@97
PS4, Line 97:   for test in plugin.tests_collected:
: print(test)
> Not sure this is necessary. It looks like you're printing tests twice: once
I have kept this to allow suppressing of pytest verbose messages and just see 
the list of collected tests by

run-tests.py shell --collect-only -p no:terminal



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:20:34 +
Gerrit-HasComments: Yes

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

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

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


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@206
PS11, Line 206: Hdfs
> I wonder if we should just drop Hdfs from this name given that this is used
yeah, I just extended the current naming. its a good point to change it so I'll 
do that since I don't see a good reason now to carry along the current naming.


http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@228
PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec 
hdfs_file_split_generator_spec
> Isn't it that the frontend generates either a list of (concrete) scan range
we currently put all scan ranges from all partitions into the same list. 
partitions may differ in their file system, so from the hdfs scan node's 
perspective, it needs to create a list that can have a mix of concrete and 
generated ranges.

we'd like to handle both cases uniformly (see L205) so doing it this way in 
this change does the least damage to the part that is not changed (concrete 
hdfs scan ranges).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:15:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Michael Brown (Code Review)]

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

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle
> The commit convention is:
Agreed. And also, please explain testing done.

For IMPALA-5886, the problem is pytest is run in several phases, and a specific 
match on a test might run a test in one of the phases only (say, just serial), 
but no tests collected during the parallel or custom cluster phases. On master, 
that results in a failure, but that's wrong. Ideally you'd test various 
scenarios like this, testing running tests in only one of the phases, to make 
sure the return code is correct. For IMPALA-4812, run-tests.py --collect-only 
should mimic impala-py.test --collect-only as much as possible.

Here's a good matrix to play with:

-m: nothing; execute_serially; "not execute_serially"

-k: nothing or some pattern match

module args: nothing or a module that has all serial, a mix of serial/parallel, 
just parallel


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@30
PS4, Line 30: from _pytest.runner import runtestprotocol
Is this used?


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
Please use new-style classes, and inherit from object.

  class TestStatisticsPlugin(object)

Should this just be call TestCounterPlugin ?


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@67
PS4, Line 67:   def __init__(self):
:   self.tests_collected = []
:   self.tests_executed = []
2 spaces, please.


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@71
PS4, Line 71: # items represents the list of collected test items
> Use Python's docstring comment instead.
Agreed.


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@72
PS4, Line 72:   def pytest_collection_modifyitems(self, items):
For these special hooks I think it's important to note that the name is 
significant and point to documentation like 
https://docs.pytest.org/en/2.9.2/writing_plugins.html#_pytest.hookspec.pytest_collection_modifyitems


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@96
PS4, Line 96: if "--collect-only" in args:
> In general, we use single quotes for strings in Python.
Unfortunately there's a mix of styles in play here. I think it's fine to use 
whatever quote system dominates the file.


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@97
PS4, Line 97:   for test in plugin.tests_collected:
: print(test)
Not sure this is necessary. It looks like you're printing tests twice: once is 
pytest doing it, the other is this print statement. I also found a bug 
somewhere:

  ./run-tests.py --collect-only -m "execute_serially" 
query_test/test_tpch_queries.py

should not select any tests because none of the test cases are serial, but your 
patch prints them.


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@100
PS4, Line 100: if exit_code == 5:
> Some documentation on what exit_code 5 means?
I wonder if this would work?
  import pytest
  # In my environment, the line above needed to run before the line below.
  from _pytest.main import EXIT_NOTESTSCOLLECTED


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@244
PS4, Line 244: print_metrics('connections')
Please make sure to maintain consistent print_metrics() usage. I think you may 
have omitted some calls that were there before.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:58:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0

[Alex Rodoni (Code Review)]

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


Change subject: IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0
..

IMPALA-3916: [DOCS] Reserved keywords updated for Impala 3.0

Change-Id: I09dbaf5e2d22f07a8bca8a601c04e94f3a4b36a0
Cherry-picks: not for 2.x
---
M docs/topics/impala_reserved_words.xml
1 file changed, 226 insertions(+), 58 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has uploaded a new patch set (#1). ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..

IMPALA-5886 Update run-tests.py script to handle exit_code for EE tests when no 
tests are collected
After this change return_code will be either 0 if no tests are expected to be 
collected (dry-run) and 1 if tests are expected to be collected but are not 
collected due
to come error

Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
---
M tests/run-tests.py
1 file changed, 19 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 1
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle exit code 5

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle
> The commit convention is:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:50:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle exit code for EE tests when no tests are collected.After this change return code will be either 0 if no tests are expecte

[Nithya Janarthanan (Code Review)]

Nithya Janarthanan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle 
exit_code for EE tests when no tests are collected.After this change 
return_code will be either 0 if no tests are expected to be collected (dry-run) 
and 1 if tests are expected to be collected
..


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
> Use 2 spaces to follow Impala Python convention.
I see what you are saying. But the other classes (Class Testexecutor) in 
run-test.py  don't follow the 2 spaces convention.

What would you recommend ?


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@71
PS4, Line 71: # items represents the list of collected test items
> Use Python's docstring comment instead.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@87
PS4, Line 87:
> Remove extra new line.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@96
PS4, Line 96: if "--collect-only" in args:
> In general, we use single quotes for strings in Python.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@100
PS4, Line 100: if exit_code == 5:
> Some documentation on what exit_code 5 means?
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@240
PS4, Line 240: collect_mode = True
> I don't see where collect_mode is declared or used.
Yes...It is not needed. It is from some debugging I was doing. removed now


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@243
PS4, Line 243:   if '--collect-only' not in sys.argv:
> Isn't it just an else statement for the if statement in line: 239?
you are right...fixed it now



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:48:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9538 )

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@80
PS1, Line 80: assert (client.get_state(short_timeout_expire_handle) ==
> These may have the same problem too, right?
my thought here is that we think we wait enough so that we're just looking up 
the current state (e.g., we're not waiting until a condition switches). so 
that's why I didn't guard these.


http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@159
PS1, Line 159: assert client.get_state(timeout_handle) == 
client.QUERY_STATES['EXCEPTION']
> These asserts have the same problem, right?
same thought process as in the previous comment. primarily, I looked for the 
wait_for_metric_value.


http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@175
PS1, Line 175:   def __expect_client_state(self, client, handle, 
expected_state, max_tries=10):
> Should we have a time bound instead of number of tries? If get_state() was
we're doing an rpc for this one so I thought it should be slow enough. since 
there are timing assumptions around the call sites, I thought it may make this 
more difficult to reason with. let me know if you're more comfortable with a 
time bound.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:41:46 +
Gerrit-HasComments: Yes

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

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

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


Patch Set 11:

(2 comments)

Had a question about the thrift, I will look at the backend side too.

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@206
PS11, Line 206: Hdfs
I wonder if we should just drop Hdfs from this name given that this is used for 
other "filesystem" like storage (i.e. S3, ADLS). Ok to ignore if you prefer to 
keep it for some reason (maybe the reason to keep it is consistency with e.g. 
HdfsScanNode, which also handles S3/ADLS since they go through the same 
FileSystem & libhdfs access path).


http://gerrit.cloudera.org:8080/#/c/8523/11/common/thrift/PlanNodes.thrift@228
PS11, Line 228: 4: optional THdfsFileSplitGeneratorSpec 
hdfs_file_split_generator_spec
Isn't it that the frontend generates either a list of (concrete) scan ranges, 
or it just gives a list of FileSplitGeneratorSpec?  i.e. I would have expected 
this to appear as the alternative to Planner.TScanRangeLocationList since the 
that is the "concrete" scan range list that this thing is going to effectively 
explode into.
i.e. it seems clearer to me to leave TScanRange as the "concrete" range that 
the coordinator will generate using this new "spec".
So what's the reasoning for putting this here instead of higher up in the 
thrift plan tree?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 11
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:34:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3866 Improve error reporting for scratch write errors

[Gabor Kaszab (Code Review)]

Hello Attila Jeges, Tim Armstrong,

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

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

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

Change subject: IMPALA-3866 Improve error reporting for scratch write errors
..

IMPALA-3866 Improve error reporting for scratch write errors

The error messages coming from DiskIoMgr::Write() are enhanced by this
change. A mapping is introduced between the errno set by open(),
fdopen(), fseek(), fwrite() or fclose() low level functions and an
error message for displaying purposes. If any of these functions
fail then the returned error message is taken from this mapping.

In addition there were two functions, NewFile() and
FileAllocateSpace() that always returned Status::OK(). I made them
void and removed the status checks from the call sites.

For testing purposes a fault injection mechanism is introduced to
simulate the cases when the above mentioned functions fail.

Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7
---
M be/src/runtime/io/CMakeLists.txt
M be/src/runtime/io/disk-io-mgr-test.cc
A be/src/runtime/io/disk-io-mgr-with-fault-injection.cc
A be/src/runtime/io/disk-io-mgr-with-fault-injection.h
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
A be/src/runtime/io/errno-to-error-status-converter.cc
A be/src/runtime/io/errno-to-error-status-converter.h
M be/src/runtime/tmp-file-mgr-internal.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
12 files changed, 488 insertions(+), 76 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7
Gerrit-Change-Number: 9420
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6602: fixes flaky expiration test
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@80
PS1, Line 80: assert (client.get_state(short_timeout_expire_handle) ==
These may have the same problem too, right?


http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@159
PS1, Line 159: assert client.get_state(timeout_handle) == 
client.QUERY_STATES['EXCEPTION']
These asserts have the same problem, right?


http://gerrit.cloudera.org:8080/#/c/9538/1/tests/custom_cluster/test_query_expiration.py@175
PS1, Line 175:   def __expect_client_state(self, client, handle, 
expected_state, max_tries=10):
Should we have a time bound instead of number of tries? If get_state() was 
sufficiently fast we'd still have the flakiness.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:28:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add fragment instance id/query id to important log messages and make them grep friendly

[Impala Public Jenkins (Code Review)]

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

Change subject: Add fragment_instance_id/query_id to important log messages and 
make them grep friendly
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Gerrit-Change-Number: 9535
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:21:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] Add fragment instance id/query id to important log messages and make them grep friendly

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Dan Hecht,

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

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

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

Change subject: Add fragment_instance_id/query_id to important log messages and 
make them grep friendly
..

Add fragment_instance_id/query_id to important log messages and make them grep 
friendly

Debugging via going through log messages can get very tricky when
the fragment_instance_id and query_id are missing from certain important
log messages. This patch tries to add it in places where I've had issues
in the past.

This patch also tries to standardize on 'fragment_instance_id' and
'query_id' over all log messages so that it's easy to grep through the
logs while debugging.

There may be more places that I've missed, but we can fix them as we
come across them.

Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
---
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
6 files changed, 37 insertions(+), 30 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Gerrit-Change-Number: 9535
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] Add fragment instance id/query id to important log messages and make them grep friendly

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9535 )

Change subject: Add fragment_instance_id/query_id to important log messages and 
make them grep friendly
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9535/1/be/src/runtime/data-stream-sender.cc
File be/src/runtime/data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/9535/1/be/src/runtime/data-stream-sender.cc@278
PS1, Line 278: :
would make more sense without this colon (like line 316)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb79c0cfa8cb8b99b8539e15e3c1adcb6854cf3
Gerrit-Change-Number: 9535
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:13:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6477: rpc-mgr-kerberized-test fails on CentOS 6.4

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6477: rpc-mgr-kerberized-test fails on CentOS 6.4
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7227551aabd1ef4f8e8608fefb74293f9f763e13
Gerrit-Change-Number: 9519
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:06:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6477: rpc-mgr-kerberized-test fails on CentOS 6.4

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6477: rpc-mgr-kerberized-test fails on CentOS 6.4
..

IMPALA-6477: rpc-mgr-kerberized-test fails on CentOS 6.4

On systems that have Kerberos 1.11 or earlier, service principals with
IP addresses are not supported due to a bug:

http://krbdev.mit.edu/rt/Ticket/Display.html?id=7603

Since our BE tests use such principals, they fail on older platforms with the
above mentioned kerberos versions.

Kudu fixed this by adding a workaround which overrides krb5_realm_override.

https://github.com/cloudera/kudu/commit/ba2ae3de4a7c43ff2f5873e822410e066ea99667

We realized that even though this is linked correctly on older platforms, it 
does
not turn on until the KUDU_ENABLE_KRB5_REALM_FIX environment variable is set.

This patch sets it only for tests. We DO NOT enable this workaround for live
clusters. The reasoning is that if a user of Impala is using an older
version of kerberos that has a known bug of not being able to handle
numeric IP addresses, then it's not on Impala to fix that issue. We allow
it for tests because we want to be able to run our tests on multiple
platforms.

Change-Id: I7227551aabd1ef4f8e8608fefb74293f9f763e13
Reviewed-on: http://gerrit.cloudera.org:8080/9519
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
2 files changed, 5 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7227551aabd1ef4f8e8608fefb74293f9f763e13
Gerrit-Change-Number: 9519
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6523: [DOCS] Allowed values for PARQUET FALLBACK SCHEMA RESOLUTION

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6523: [DOCS] Allowed values for 
PARQUET_FALLBACK_SCHEMA_RESOLUTION
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ade6a30c62f4899b6801d03430b5fe15e713474
Gerrit-Change-Number: 9536
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:01:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6605: Exception hidden on complex types

[Adam Holley (Code Review)]

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

Change subject: IMPALA-6605: Exception hidden on complex types
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ae62fe63b82899d1467a23422d4a73f1471aa84
Gerrit-Change-Number: 9514
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 07 Mar 2018 22:00:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6602: fixes flaky expiration test

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9538


Change subject: IMPALA-6602: fixes flaky expiration test
..

IMPALA-6602: fixes flaky expiration test

The test_query_expiration test assumes that a metric and
the query state are maintained atomically. Since they're
not, occasionaly flakes (false negatives) occur.

The fix in this patch is to loop until the expected state
is seen. If the expected state is not seen with a given number
of iterations, the test fails. These tests depend on timing so
if this validation takes too long, the test will also fail.
Such looping is used in the two places where its assumed that the
client state and metrics are maintained atomically.

Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
---
M tests/custom_cluster/test_query_expiration.py
1 file changed, 17 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Gerrit-Change-Number: 9538
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac