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

2018-03-06 Thread 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:

(1 comment)

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@197
PS1, Line 197:   /// 'recvr_side_profile_' and 'sender_side_profile_'. Not 
owned.
> It's pretty critical that 'recvr_side_profile_' and "sender_side_profile_'
I believe so by the virtue that the receiver is co-owned by the exchange node 
and the data stream manager. So, the receiver will have a lifetime of at least 
as long as the exchange node's.

'profile_' and the exchange node are both owned by the RuntimeState's 'pool_' 
so 'profile_' lifetime should be no longer than that of the exchange node.

Combining the above and the fact that 'recvr_side_profile_' and 
'sender_side_profile_'  are owned by the receiver, they should have a lifetime 
at least as long as 'profile_'.

Will add a comment about this requirement in PS2.



--
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: comment
Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a
Gerrit-Change-Number: 9527
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Mar 2018 07:16:41 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong 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:

(1 comment)

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@197
PS1, Line 197:   /// 'recvr_side_profile_' and 'sender_side_profile_'. Not 
owned.
It's pretty critical that 'recvr_side_profile_' and "sender_side_profile_' 
outlive 'profile_', so that top-down walking of the RuntimeProfile doesn't 
reference invalid memory. Is this guaranteed? We should document this 
requirement in this comment regardless.



--
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: comment
Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a
Gerrit-Change-Number: 9527
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Mar 2018 06:29:08 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9527


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/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
2 files changed, 34 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/9527/1
--
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: newchange
Gerrit-Change-Id: I3378496e2201b16c662b9daa634333480705c61a
Gerrit-Change-Number: 9527
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

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

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 14: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 14
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 03:03:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

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

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..

IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

On systems with slow networking large queuing can occur in the Reactor
threads. It would be good to quantify how much queueing occurred.

This patch extracts the OutboundTransfer queue size information from
KRPC and displays it on the /rpcz webpage.

Testing: Tested by running queries both on a local mini-cluster and a
remote cluster and manually confirming from the webpages that connections
are made between the different nodes that exchange with each other and show
number of queued calls.

Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Reviewed-on: http://gerrit.cloudera.org:8080/9384
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/rpc/rpc-mgr.cc
M www/rpcz.tmpl
2 files changed, 48 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 15
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-6389: Make '\0' delimited text files work

2018-03-06 Thread Zach Amsden (Code Review)
Zach Amsden has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9525


Change subject: IMPALA-6389: Make '\0' delimited text files work
..

IMPALA-6389: Make '\0' delimited text files work

This is conflated a bit with the support work required to get
MySQL to run in the mini-cluster to support HMS, as Postgresql
simply rejects these table creations outright.

The basic idea is to specialize the class on whether row/tuple
delimiters are actually used, as the use case for sequence and
text files is quite different.  Unfortunately, that came out a
bit more ugly than I would have liked, as the class template
parameter infected all definitions.

Testing: Created a zero delimited table as described in the JIRA,
ran select * from tab_separated on the table.

Change-Id: I4b6f38cbe3f1036f60efd31a31d82d0cd8f3d2a8
---
M be/src/exec/delimited-text-parser.cc
M be/src/exec/delimited-text-parser.h
M be/src/exec/delimited-text-parser.inline.h
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/exec/hdfs-sequence-scanner.h
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/hdfs-text-scanner.h
M bin/create-test-configuration.sh
M bin/impala-config.sh
M buildall.sh
M fe/pom.xml
M testdata/bin/run-sentry-service.sh
12 files changed, 136 insertions(+), 41 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4b6f38cbe3f1036f60efd31a31d82d0cd8f3d2a8
Gerrit-Change-Number: 9525
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden 


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

2018-03-06 Thread 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 3:

(1 comment)

Thank you Fredy, I have solved that problem.

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

http://gerrit.cloudera.org:8080/#/c/9506/1/shell/impala_shell.py@1539
PS1, Line 1539:   if p.returncode != 0:
> You should only strip the new line when there's no error. Move line: 1539 t
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: 3
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:44:52 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread 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 (#3).

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_cmd Impala shell get contains extra line break.

I fix this issue by removing extraneous '\n' in value when impala-shell using
ldap_password_cmd.

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/3
--
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: 3
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 


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

2018-03-06 Thread 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 (#2).

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_cmd Impala shell get contains extra line break.

I fix this issue by removing extraneous '\n' in value when impala-shell using
ldap_password_cmd.

Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
---
M shell/impala_shell.py
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/9506/2
--
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: 2
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-5903: Inconsistent specification of result set and result set metadata

2018-03-06 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9090 )

Change subject: IMPALA-5903: Inconsistent specification of result set and 
result set metadata
..


Patch Set 3:

Now all DDL operations return a result set.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
Gerrit-Change-Number: 9090
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:27:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5903: Inconsistent specification of result set and result set metadata

2018-03-06 Thread Zoltan Borok-Nagy (Code Review)
Hello Dimitris Tsirogiannis, Tim Armstrong,

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

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

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

Change subject: IMPALA-5903: Inconsistent specification of result set and 
result set metadata
..

IMPALA-5903: Inconsistent specification of result set and result set metadata

Before this commit it was quite random which DDL oprations
returned a result set and which didn't.

With this commit, every DDL operations return a summary of
its execution. They declare their result set schema in
Frontend.java, and provide the summary in CalatopOpExecutor.java.

Updated the tests according to the new behavior.

Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
---
M be/src/service/client-request-state.cc
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M testdata/workloads/functional-query/queries/QueryTest/chars-tmp-tables.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
M testdata/workloads/functional-query/queries/QueryTest/create-database.test
M 
testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
M 
testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
M testdata/workloads/functional-query/queries/QueryTest/create-table.test
M 
testdata/workloads/functional-query/queries/QueryTest/delimited-latin-text.test
M testdata/workloads/functional-query/queries/QueryTest/delimited-text.test
M testdata/workloads/functional-query/queries/QueryTest/describe-path.test
M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test
M 
testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/insert_bad_expr.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
M testdata/workloads/functional-query/queries/QueryTest/load.test
M testdata/workloads/functional-query/queries/QueryTest/local-filesystem.test
M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
24 files changed, 273 insertions(+), 64 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
Gerrit-Change-Number: 9090
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


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

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8936 )

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


Patch Set 7:

I'm still seeing crashes when I loop this locally:
 F0306 16:34:04.306205 29911 mem-pool.h:245] Check failed: size >= 0 (-9 vs. 0)
*** Check failure stack trace: ***
@  0x3c32cbd  google::LogMessage::Fail()
@  0x3c34562  google::LogMessage::SendToLog()
@  0x3c32697  google::LogMessage::Flush()
@  0x3c35c5e  google::LogMessageFatal::~LogMessageFatal()
@  0x184eeb4  impala::MemPool::TryAllocate()
@  0x1be5d08  impala::HdfsRCFileScanner::StartRowGroup()
@  0x1be8032  impala::HdfsRCFileScanner::ProcessRange()
@  0x296931a  impala::BaseSequenceScanner::GetNextInternal()
@  0x1bc71b2  impala::HdfsScanner::ProcessSplit()
@  0x1b9f48c  impala::HdfsScanNode::ProcessSplit()
@  0x1b9e8b7  impala::HdfsScanNode::ScannerThread()
@  0x1b9dd3f  
_ZZN6impala12HdfsScanNode22ThreadTokenAvailableCbEPNS_17ThreadResourceMgr12ResourcePoolEENKUlvE_clEv
@  0x1b9fcdd  
_ZN5boost6detail8function26void_function_obj_invoker0IZN6impala12HdfsScanNode22ThreadTokenAvailableCbEPNS3_17ThreadResourceMgr12ResourcePoolEEUlvE_vE6invokeERNS1_15function_bufferE
@  0x17f4ce0  boost::function0<>::operator()()
@  0x1af63ff  impala::Thread::SuperviseThread()
@  0x1aff3cf  boost::_bi::list5<>::operator()<>()
@  0x1aff2f3  boost::_bi::bind_t<>::operator()()
@  0x1aff2b6  boost::detail::thread_data<>::run()
@  0x2dbc1aa  thread_proxy
@ 0x7f5af8f926ba  start_thread
@ 0x7f5af8cc841d  clone
Picked up JAVA_TOOL_OPTIONS: 
-agentlib:jdwp=transport=dt_socket,address=3,server=y,suspend=n
Wrote minidump to 
/home/tarmstrong/Impala/incubator-impala/logs/cluster/minidumps/impalad/31a8ae22-6a2a-4118-4ff4e1b6-be18ef7c.dmp

How about we leave the test disabled and move forward with the improvements, 
then we can finish up the remaining fixes needed later?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 8936
Gerrit-PatchSet: 7
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 07 Mar 2018 01:24:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5903: Inconsistent specification of result set and result set metadata

2018-03-06 Thread Zoltan Borok-Nagy (Code Review)
Hello Dimitris Tsirogiannis, Tim Armstrong,

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

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

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

Change subject: IMPALA-5903: Inconsistent specification of result set and 
result set metadata
..

IMPALA-5903: Inconsistent specification of result set and result set metadata

Before this commit it was quite random which DDL oprations
returned a result set and which didn't.

With this commit, every DDL operations return a summary of
its execution. They declare their result set schema in
Frontend.java, and provide the summary in CalatopOpExecutor.java.

Updated the tests according to the new behavior.

Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
---
M be/src/service/client-request-state.cc
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M testdata/workloads/functional-query/queries/QueryTest/chars-tmp-tables.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
M testdata/workloads/functional-query/queries/QueryTest/create-database.test
M 
testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
M 
testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
M testdata/workloads/functional-query/queries/QueryTest/create-table.test
M 
testdata/workloads/functional-query/queries/QueryTest/delimited-latin-text.test
M testdata/workloads/functional-query/queries/QueryTest/delimited-text.test
M testdata/workloads/functional-query/queries/QueryTest/describe-path.test
M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test
M 
testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/insert_bad_expr.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test
M testdata/workloads/functional-query/queries/QueryTest/load.test
M testdata/workloads/functional-query/queries/QueryTest/local-filesystem.test
M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test
M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
24 files changed, 272 insertions(+), 64 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic542fb8e49e850052416ac663ee329ee3974e3b9
Gerrit-Change-Number: 9090
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@588
PS9, Line 588: for index in xrange(0, len(parsed_statements)):
> Isn't xrange is faster in Python 2.7 compared to enumerate?
It probably is faster, but I don't think performance should be a concern in 
this case.

If you want to keep xrange(), then you can get rid of the first argument. 
xrange(len(parsed_statements))



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:24:20 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9501 )

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


Patch Set 2:

(8 comments)

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

http://gerrit.cloudera.org:8080/#/c/9501/2//COMMIT_MSG@11
PS2, Line 11: for a future change
Do we have a JIRA for this?


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

http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@344
PS2, Line 344:   /// We enforce the invariant that query_status_ is not OK iff 
operation_state_ is
nit: add line


http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@345
PS2, Line 345: query_state_
operation_state_


http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@405
PS2, Line 405: status_
query_status_


http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@405
PS2, Line 405: query_state_
operation_state_


http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@408
PS2, Line 408: status_
query_status_


http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.h@450
PS2, Line 450: apache::hive::service::cli::thrift::
Are we against adding "using namespace ..." in a header file?


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

http://gerrit.cloudera.org:8080/#/c/9501/2/be/src/service/client-request-state.cc@1109
PS2, Line 1109: "Query State"
Do we want to leave this as "Query State" after deprecating beeswax? Or change 
it to "Operation State" ?



--
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: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:24:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancing Algorithm topic

2018-03-06 Thread John Russell (Code Review)
John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancing Algorithm topic
..


Patch Set 1:

(1 comment)

Just an FYI to Alex's response on Alan's question.

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@162
PS1, Line 162:   
> John Russel was waiting for reviews.
Yes, it slipped through the cracks while moving doc development to upstream. 
Especially because if we started going into a lot of detail about using Impala 
with , that could arguably be inappropriate to 
include in upstream docs.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:23:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@588
PS9, Line 588: for index in xrange(0, len(parsed_statements)):
> enumerate*
Isn't xrange is faster in Python 2.7 compared to enumerate?


http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601
PS9, Line 601: if not found_error:
> I tried it, it looks like it throws away the middle "not error" statement.
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:20:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601
PS9, Line 601: if not found_error:
> Ok, but what will happen if the list looks like the following:
I tried it, it looks like it throws away the middle "not error" statement. I 
think that's weird behavior.

I tried it with this query in impala-shell:
select 1; \; select 2; \; select 3;

"Select 2" vanishes



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:16:47 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho 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 2: Code-Review+2

Please do some basic sanity tests on Centos 6.4 for other BE tests which also 
use min-kdc-wrapper.


--
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: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:15:17 +
Gerrit-HasComments: No


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

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil 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 1:

(2 comments)

> (2 comments)
 >
 > So, thrift-server-test didn't need this workaround because it was
 > using "impala/localhost" instead of "impala/127.0.0.1", right ?

Yes, that's right. We use a non-numeric principal for thrift-server-test, so 
the bug isn't exposed.

http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc
File be/src/testutil/mini-kdc-wrapper.cc:

http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc@69
PS1, Line 69:  // Enable the workaround for MIT krb5 1.10 bugs from 
krb5_realm_override.cc.
> nit: indent off
Done


http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc@70
PS1, Line 70: "true"
> My understanding is that the workaround will be in effect as long as this e
Yes, that's right



--
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: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 07 Mar 2018 00:09:45 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Sailesh Mukil (Code Review)
Hello Michael Ho,

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

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

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

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
---
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
2 files changed, 5 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/9519/2
--
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: newpatchset
Gerrit-Change-Id: I7227551aabd1ef4f8e8608fefb74293f9f763e13
Gerrit-Change-Number: 9519
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 


[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

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya 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:

(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:

IMPALA-XXX: Brief description

Detailed description.


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.


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.


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


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@88
PS4, Line 88: plugin = TestStatisticsPlugin()
Since we may be adding a new plugin in the future, it's probably better to 
inline TestStatisticsPlugin in line: 91.


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.


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?


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@232
PS4, Line 232:   for i in range(1,len(sys.argv)):
What's the purpose of copying all elements into a new list?


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.


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?



--
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 00:07:06 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho 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 1:

(2 comments)

So, thrift-server-test didn't need this workaround because it was using 
"impala/localhost" instead of "impala/127.0.0.1", right ?

http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc
File be/src/testutil/mini-kdc-wrapper.cc:

http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc@69
PS1, Line 69:  // Enable the workaround for MIT krb5 1.10 bugs from 
krb5_realm_override.cc.
nit: indent off


http://gerrit.cloudera.org:8080/#/c/9519/1/be/src/testutil/mini-kdc-wrapper.cc@70
PS1, Line 70: "true"
My understanding is that the workaround will be in effect as long as this 
environment is set, right ?



--
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: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:51:23 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@588
PS9, Line 588: for index in xrange(0, len(parsed_statements)):
> for index, statement in enumate(parsed_statements)
enumerate*



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:45:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@588
PS9, Line 588: for index in xrange(0, len(parsed_statements)):
for index, statement in enumate(parsed_statements)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:44:28 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601
PS9, Line 601: if not found_error:
> When found_error is true at the very end (implied by having non empty joine
Ok, but what will happen if the list looks like the following:

not error,
error,
not error,
error,
not error

Also, maybe add this to the test cases?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:42:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..

IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC

This change renames the TEST_KRPC environment variable to DISABLE_KRPC
to reflect the fact that KRPC is now enabled by default.

Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Reviewed-on: http://gerrit.cloudera.org:8080/9516
Reviewed-by: Dan Hecht 
Reviewed-by: Michael Ho 
Tested-by: Lars Volker 
---
M bin/run-all-tests.sh
1 file changed, 4 insertions(+), 5 deletions(-)

Approvals:
  Dan Hecht: Looks good to me, approved
  Michael Ho: Looks good to me, approved
  Lars Volker: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..


Patch Set 2: Verified+1

This change does not affect the builds in out GDV environment. After checking 
with Dan I'm marking it as +1 Verified.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:39:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 10: Code-Review+1

Carrying Vuk's +1.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:33:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..

IMPALA-6337: Fix infinite loop in Impala shell

The patch uses a workaround for a bug in the sqlparse.split() function
by joining the statements that contain error tokens into a single
statement

Testing:
- Ran end-to-end shell tests

Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 59 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 


[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

2018-03-06 Thread Nithya Janarthanan (Code Review)
Nithya Janarthanan has uploaded this change for review. ( 
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
..

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 but are not collected due to some error

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



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/9494/4
--
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: newchange
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 4
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Nithya Janarthanan 


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancing Algorithm topic

2018-03-06 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancing Algorithm topic
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9515/3//COMMIT_MSG@7
PS3, Line 7: [DOCS] Publish Choosing the Load-Balancign Algorithm
> Typo in balancing.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:27:41 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancing Algorithm topic

2018-03-06 Thread Alex Rodoni (Code Review)
Hello John Russell, Fredy Wijaya, Alan Choi,

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

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

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

Change subject: [DOCS] Publish Choosing the Load-Balancing Algorithm topic
..

[DOCS] Publish Choosing the Load-Balancing Algorithm topic

Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
---
M docs/topics/impala_proxy.xml
1 file changed, 3 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: John Russell 


[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/util/rle-encoding.h@674
PS3, Line 674:memset(values + num_consumed, repeated_value, 
num_repeats_to_set);
> It's not valid to use memset() here in general since sizeof(T) may be > 1.
We could also be lazy and not implement it for sizeof(T) > 1 until we actually 
need it, but I think we should just fix it properly while we're here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:21:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

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

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 14:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 14
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: Tue, 06 Mar 2018 23:20:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 14: Code-Review+2

(1 comment)

Rebase, Carry +2.

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@231
PS11, Line 231: [[row["remote_ip"], row["outbound_queue_size"
> nit: I meant to keep the map(..) syntax since it's more concise and just re
Ah understood. I changed it back to the map(..), with the explicit key ordering.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 14
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:20:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 3:

(7 comments)

The overall approach looks good, had some comments about the details and the 
test coverage.

http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/exec/parquet-column-readers.cc@673
PS3, Line 673: switch(page_encoding_) {
nit: space before (


http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/exec/parquet-column-readers.cc@1304
PS3, Line 1304:   stringstream ss;
Convert to using Substitute() while you're here - we generally prefer using it 
over stringstream.


http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/util/rle-encoding.h@138
PS3, Line 138:   /// Returns the number of consumed values or 0 if an error 
occurred.
Can you add some tests to be/src/util/rle-test.cc for the new interface? There 
are already some tests there that exercise both GetSingleValue() and the 
NextNum*/Get*Values() interfaces, so we could also add a test for this variant 
of the interface.


http://gerrit.cloudera.org:8080/#/c/9403/3/be/src/util/rle-encoding.h@674
PS3, Line 674:memset(values + num_consumed, repeated_value, 
num_repeats_to_set);
It's not valid to use memset() here in general since sizeof(T) may be > 1. The 
reason it worked for the Parquet level decoding was because they were always a 
uint8_t.

In general we need a for loop here to do it correctly, but my understanding was 
that memset() is sometimes faster - it was used as an optimisation. I see two 
paths forward here:
1. maybe gcc converts the for loop to a memset() anyway when sizeof(T) is 1 
bytes (at least some compilers can do this conversion if the code fits the 
right pattern).
2. do an if(sizeof(T) == 1) here so that we can use the optimized memset() path 
in that case, then fall back to a for loop otherwise.


http://gerrit.cloudera.org:8080/#/c/9403/3/testdata/data/README
File testdata/data/README:

http://gerrit.cloudera.org:8080/#/c/9403/3/testdata/data/README@160
PS3, Line 160: Parquet v1 file with a single RLE encoded boolean column "b". 
Created for IMPALA-6324.
How was it created? Parquet-mr?


http://gerrit.cloudera.org:8080/#/c/9403/3/testdata/workloads/functional-query/queries/QueryTest/parquet-rle-encoded-bool.test
File 
testdata/workloads/functional-query/queries/QueryTest/parquet-rle-encoded-bool.test:

http://gerrit.cloudera.org:8080/#/c/9403/3/testdata/workloads/functional-query/queries/QueryTest/parquet-rle-encoded-bool.test@4
PS3, Line 4: select count(*) from rle_encoded_bool where b;
This wouldn't detect if the boolean values were returned in the wrong order. 
E.g. if we somehow got the bit-packing backwards and the values were in the 
wrong order (we had a long-standing bug with the BIT_PACKED levels).

Can we also add a test with two columns (e.g. id and boolean) and make sure 
that they match up correctly.


http://gerrit.cloudera.org:8080/#/c/9403/3/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/9403/3/tests/query_test/test_scanners.py@656
PS3, Line 656: """
Nit: trailing """ can go on same line.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:20:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread 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/9384

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

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..

IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

On systems with slow networking large queuing can occur in the Reactor
threads. It would be good to quantify how much queueing occurred.

This patch extracts the OutboundTransfer queue size information from
KRPC and displays it on the /rpcz webpage.

Testing: Tested by running queries both on a local mini-cluster and a
remote cluster and manually confirming from the webpages that connections
are made between the different nodes that exchange with each other and show
number of queued calls.

Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
---
M be/src/rpc/rpc-mgr.cc
M www/rpcz.tmpl
2 files changed, 48 insertions(+), 1 deletion(-)


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

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


[Impala-ASF-CR] IMPALA-6605: Exception hidden on complex types

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9514 )

Change subject: IMPALA-6605: Exception hidden on complex types
..

IMPALA-6605: Exception hidden on complex types

This patch fixes the issue when we have a column privilege on a
particular table, but the statement has an analysis error.

Example:
> grant select(int_struct_col) on table functional.allcomplextypes
> to role test_role;

> select 1 from functional.allcomplextypes.int_struct_col;

The select statement above should throw an AnalysisException and

AuthorizationException since we have int_struct_col column
ege on
functional.allcomplextypes table.

Testing:
- Ran all front-end tests

Change-Id: I3ae62fe63b82899d1467a23422d4a73f1471aa84
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/common/AnalysisException.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
4 files changed, 79 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/9514/4
--
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: newpatchset
Gerrit-Change-Id: I3ae62fe63b82899d1467a23422d4a73f1471aa84
Gerrit-Change-Number: 9514
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 13: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@231
PS11, Line 231: = [];
> Done
nit: I meant to keep the map(..) syntax since it's more concise and just 
replace the return.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 13
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:06:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9515/3//COMMIT_MSG@7
PS3, Line 7: [DOCS] Publish Choosing the Load-Balancign Algorithm
Typo in balancing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:06:15 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread 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 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9509/1//COMMIT_MSG@11
PS1, Line 11: statement.
It's good to give some examples on where the inconsistencies are.


http://gerrit.cloudera.org:8080/#/c/9509/1/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/1/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java@47
PS1, Line 47:   // Flag for controlling whether last node in the path is 
visible.
Why is this not private?


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@52
PS1, Line 52:
Remove extra new line.


http://gerrit.cloudera.org:8080/#/c/9509/1/fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java@62
PS1, Line 62: if (isLastNodeVisible()) {
By looking at this implementation, last node is always the column. Isn't it 
better to rename the method as isColumnVisible() instead?



--
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: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 06 Mar 2018 23:04:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@173
PS1, Line 173: leastconn
> This makes sense for Impala. We recommend leastconn for F5.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:44:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alex Rodoni (Code Review)
Hello John Russell, Alan Choi,

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

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

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

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..

[DOCS] Publish Choosing the Load-Balancign Algorithm

Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
---
M docs/topics/impala_proxy.xml
1 file changed, 3 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alex Rodoni (Code Review)
Hello John Russell, Alan Choi,

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

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

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

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..

[DOCS] Publish Choosing the Load-Balancign Algorithm

Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
---
M docs/topics/impala_proxy.xml
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: John Russell 


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@162
PS1, Line 162:   
> Do you happen to know why it was hidden to begin with?
John Russel was waiting for reviews.


http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@195
PS1, Line 195: Recommended for use with Hue.
> Hue requires sticky, not "source affinity".
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:42:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alan Choi (Code Review)
Alan Choi has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9515 )

Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@162
PS1, Line 162:   
Do you happen to know why it was hidden to begin with?


http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@173
PS1, Line 173: leastconn
This makes sense for Impala. We recommend leastconn for F5.
http://www.cloudera.com/documentation/other/reference-architecture/PDF/Impala-HA-with-F5-BIG-IP.pdf


http://gerrit.cloudera.org:8080/#/c/9515/1/docs/topics/impala_proxy.xml@195
PS1, Line 195: Recommended for use with Hue.
Hue requires sticky, not "source affinity".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
Gerrit-Change-Number: 9515
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:32:57 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has uploaded this change for review. ( 
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
---
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/testutil/mini-kdc-wrapper.cc
2 files changed, 5 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/9519/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: newchange
Gerrit-Change-Id: I7227551aabd1ef4f8e8608fefb74293f9f763e13
Gerrit-Change-Number: 9519
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-03-06 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9195 )

Change subject: IMPALA-6337: Fix infinite loop in Impala shell
..


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@590
PS9, Line 590: filter
It's not very Pythonic to use the filter function. The following is much more 
Pythonic:
error_count = len([tok for tok in statement.tokens if tok.ttype == 
tokens.Error])


http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601
PS9, Line 601: if not found_error:
What if found error is true here? Then something must have gone wrong, right? 
Maybe return split_statements in that case?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b
Gerrit-Change-Number: 9195
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:12:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 13: Code-Review+1

I will let Lars take another look.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 13
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:06:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@230
PS11, Line 230: $.map(json["per_conn_metrics"], function(row) {
  : return [$.map(row, function(cell) { return cell; })];
> The 'keys' var is what we use to index into the JSON. If we add/remove a fi
If we remove fields being presented in the table or add new fields to the 
table, we would have to update the table layout anyway. So, I don't see it 
being a big deal to also update this function.

IMHO, the code is much easier to follow if we explicitly specify the keys to 
index into JSON than having the ordering of the fields implicitly encoded in 
rpc-mgr.cc.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 11
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:03:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 22:00:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:56:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..


Patch Set 1:

(1 comment)

Thanks for the review, Sailesh. Please see PS2.

http://gerrit.cloudera.org:8080/#/c/9516/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/9516/1/bin/run-all-tests.sh@41
PS1, Line 41: (default: false)
> nit: not needed, it's pretty obvious from the next line.
Good point. I moved it below to the parameters, and reworded the comment to 
point out that it affects both the cluster startup and test execution.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:55:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Lars Volker (Code Review)
Hello Michael Ho, Philip Zeyliger, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..

IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC

This change renames the TEST_KRPC environment variable to DISABLE_KRPC
to reflect the fact that KRPC is now enabled by default.

Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
---
M bin/run-all-tests.sh
1 file changed, 4 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9516 )

Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9516/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/9516/1/bin/run-all-tests.sh@41
PS1, Line 41: (default: false)
nit: not needed, it's pretty obvious from the next line.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
Gerrit-Change-Number: 9516
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:52:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 13:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@231
PS11, Line 231: = [];
> I think you could just replace this with
Done


http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@230
PS11, Line 230:
  :   var rows = [];
> Not sure I understand the comment about why we need to update this function
The 'keys' var is what we use to index into the JSON. If we add/remove a field, 
then we need to update the 'keys' list.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 13
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:47:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread 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/9384

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

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..

IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

On systems with slow networking large queuing can occur in the Reactor
threads. It would be good to quantify how much queueing occurred.

This patch extracts the OutboundTransfer queue size information from
KRPC and displays it on the /rpcz webpage.

Testing: Tested by running queries both on a local mini-cluster and a
remote cluster and manually confirming from the webpages that connections
are made between the different nodes that exchange with each other and show
number of queued calls.

Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
---
M be/src/rpc/rpc-mgr.cc
M www/rpcz.tmpl
2 files changed, 50 insertions(+), 1 deletion(-)


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

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


[Impala-ASF-CR] IMPALA-6394: Restart HDFS when blocks are under replicated

2018-03-06 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9469 )

Change subject: IMPALA-6394: Restart HDFS when blocks are under replicated
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9469/3/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

http://gerrit.cloudera.org:8080/#/c/9469/3/testdata/bin/create-load-data.sh@462
PS3, Line 462: if [[ "$RESTART_COUNT" -eq "$MAX_RETRIES" ]] ; then
We only enter the loop body when RESTART_COUNT < MAX_RETRIES, so this condition 
can never be satisfied.


http://gerrit.cloudera.org:8080/#/c/9469/3/testdata/bin/create-load-data.sh@469
PS3, Line 469: ${IMPALA_HOME}/testdata/bin/run-mini-dfs.sh
Let's echo a message that there are underreplicated blocks and that we will 
restart HDFS to try to resolve that issue.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iefd4c2fc6c287f054e385de52bdc42b0bdbd7915
Gerrit-Change-Number: 9469
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:41:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6613: Change TEST KRPC to DISABLE KRPC

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9516


Change subject: IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC
..

IMPALA-6613: Change TEST_KRPC to DISABLE_KRPC

This change renames the TEST_KRPC environment variable to DISABLE_KRPC
to reflect the fact that KRPC is now enabled by default.

Change-Id: I2081d9b53566607c25bcf84c4c92f13ee93be162
---
M bin/run-all-tests.sh
1 file changed, 4 insertions(+), 4 deletions(-)



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

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


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

2018-03-06 Thread Dan Hecht (Code Review)
Dan Hecht has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9501


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

IMPALA-6614: ClientRequestState should use HS2 TOperationState

Currently it uses beeswax's QueryState enum, but the TOperationState
is a superset. In order to remove dependencies on beeswax, and also
set things up for a future change to use the TOperationState explicit
CANCELED_STATE, migrate CLR to use TOperationState.

The intent of this change is to make no client visible change.

Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
6 files changed, 78 insertions(+), 65 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/9501/2
--
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: newchange
Gerrit-Change-Id: I36287eaf8f1dac23c306b470f95f379dfdc6bb5b
Gerrit-Change-Number: 9501
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 


[Impala-ASF-CR] [DOCS] Publish Choosing the Load-Balancign Algorithm

2018-03-06 Thread Alex Rodoni (Code Review)
Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9515


Change subject: [DOCS] Publish Choosing the Load-Balancign Algorithm
..

[DOCS] Publish Choosing the Load-Balancign Algorithm

Change-Id: I7c29ddac53c0fb4cc6a29701e50280b95167
---
M docs/topics/impala_proxy.xml
1 file changed, 1 insertion(+), 1 deletion(-)



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

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


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
> We could have some tests like that, but we would still have to test the "va
Yeah I guess there's a little bit of work to adapt the query syntax. Anyway, 
would be worth filing a JIRA to look into it.


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

http://gerrit.cloudera.org:8080/#/c/9508/2/be/src/exprs/expr-test.cc@6545
PS2, Line 6545:   TestValue("extract(cast('2006-05-12 18:27:28.12345' as 
timestamp), 'MoNTH')",
I also confirmed that we don't support MILLENNIUM/CENTURY/DECADE for extract


http://gerrit.cloudera.org:8080/#/c/9508/2/be/src/exprs/expr-test.cc@8350
PS2, Line 8350:   TestTimestampValue("date_trunc('DECADE', '2116-05-08 
10:30:00')",
I also checked DECADE against postgres


http://gerrit.cloudera.org:8080/#/c/9508/2/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

http://gerrit.cloudera.org:8080/#/c/9508/2/be/src/exprs/udf-builtins.cc@161
PS2, Line 161:   } else {
Can you add a comment that decades are considered to start with years ending in 
'0'? Otherwise it looks somewhat inconsistent.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:24:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

2018-03-06 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9293 )

Change subject: IMPALA-6515: [DOCS] HAproxy with sticky session requires the 
check option
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9293/3/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9293/3/docs/topics/impala_proxy.xml@493
PS3, Line 493: an error when the Impalad server that Hue tries to 
connect is down.
> It's not clear why it is applicable to Hue.
So is it not Hue that requires the check option?
Then, can I just remove "Hue requires" and replace with "Add the check option 
at the end of each line"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
Gerrit-Change-Number: 9293
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 21:05:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6605: Exception hidden on complex types

2018-03-06 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9514


Change subject: IMPALA-6605: Exception hidden on complex types
..

IMPALA-6605: Exception hidden on complex types

This patch fixes the issue when we have a column privilege on a
particular table, but the statement has an analysis error.

Example:
> grant select(int_struct_col) on table functional.allcomplextypes
> to role test_role;

> select 1 from functional.allcomplextypes.int_struct_col;

The select statement above should throw an AnalysisException and not an
AuthorizationException since we have int_struct_col column privilege on
functional.allcomplextypes table.

Testing:
- Ran all front-end tests

Change-Id: I3ae62fe63b82899d1467a23422d4a73f1471aa84
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/common/AnalysisException.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
4 files changed, 78 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/9514/3
--
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: newchange
Gerrit-Change-Id: I3ae62fe63b82899d1467a23422d4a73f1471aa84
Gerrit-Change-Number: 9514
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

2018-03-06 Thread Alan Choi (Code Review)
Alan Choi has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9293 )

Change subject: IMPALA-6515: [DOCS] HAproxy with sticky session requires the 
check option
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9293/3/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9293/3/docs/topics/impala_proxy.xml@493
PS3, Line 493: an error when the Impalad server that Hue tries to 
connect is down.
> Did you get a chance to address this comment? If we don't know any caveats,
It's not clear why it is applicable to Hue.

>From what I read, this will force HAProxy to check the host before making a 
>connection by making a TCP connection. Does the benefit justify the slight 
>delay? Or should the check be done at the user level (since most likely Impala 
>is not down)?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
Gerrit-Change-Number: 9293
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:59:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 2: Code-Review+1

Tim, mind taking a look too?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:24:55 +
Gerrit-HasComments: No


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

2018-03-06 Thread 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 1:

I am not sure how valid the results/reproducible are but I ran a perf test on a 
single host and here are the results: 
https://drive.google.com/file/d/1SR7SPQ8Hd_pGJ22QSL1kP9nflCFn8Ql9/view?usp=sharing

I haven't analyzed what could cause the slowdown in the 2 or  3 queries most 
impacted yet. It could be min/max filters or perhaps more expensive algebra 
functions.


--
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: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:10:01 +
Gerrit-HasComments: No


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

2018-03-06 Thread 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 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9506/1/shell/impala_shell.py@1539
PS1, Line 1539:   options.ldap_password = options.ldap_password.strip('\n')
You should only strip the new line when there's no error. Move line: 1539 to 
line: 1544.



--
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: 1
Gerrit-Owner: Donghui Xu 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 06 Mar 2018 20:00:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Attila Jeges (Code Review)
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 2:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
> please double check the behavior of DECADE, CENTURY and MILLENNIUM against
Done


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
> We already depend on PostgreSQL in development environments because of the
We could have some tests like that, but we would still have to test the "valid 
input - invalid output" cases separately as the range of TIMESTAMP values 
supported by PostgreSQL and Impala is different. The SQL syntax they use is 
also slightly different (PostgreSQL needs explicit type cast to TIMESTAMP).


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@144
PS1, Line 144: n
> missing an 'n' in spelling of Millennium
Done


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@145
PS1, Line 145:   DCHECK_GT(orig_date.year(), 2000);
> maybe add a quick comment explaining this, from your commit message:
Done


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@150
PS1, Line 150: }
> I think this handles both cases?
It does, thanks.


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@163
PS1, Line 163:   date new_date(orig_date.year() / 10 * 10, 1, 1);
> same comments
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:58:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Attila Jeges (Code Review)
Attila Jeges has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..

IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY precision

This change fixes the following correctness bug: date_trunc() function
returns the wrong result when truncating a TIMESTAMP value to
'MILLENNIUM' precision. E.g.:

select now(), date_trunc('millennium', now());
+---+-+
| now() | date_trunc('millennium', now()) |
+---+-+
| 2017-12-05 14:00:30.296812000 | 2000-01-01 00:00:00 |
+---+-+

The first year of the current millennium should be 2001.

'CENTURY' precision has the same issue.

Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins.cc
2 files changed, 43 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@231
PS11, Line 231: i = 0; i < json["per_conn_metrics"].length; +
I think you could just replace this with

  return [row["remote_ip"], row["outbound_queue_size"]];

That does all you do in 6 lines in PS12, at the cost of duplicating "row". I 
think the requirement to keep this in sync with the HTML is fine. We do it 
everywhere else in this file and making this whole thing totally generic might 
not be worth the added complexity, given how rarely we update it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 12
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:58:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

2018-03-06 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@230
PS11, Line 230:
  :   for (var i = 0; i < json["per_conn_metrics"].length; ++
> I tried changing the order of the JSON and it turns out that it does depend
Not sure I understand the comment about why we need to update this function 
when we add/remove fields in the json output ?  Isn't the point of doing 
conn_json[key] to prevent this need to update this function unless we change 
the table layout in which case it makes sense to update this function ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 12
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Mar 2018 19:37:33 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Zoltan Borok-Nagy (Code Review)
Hello Lars Volker, Gabor Kaszab, Jim Apple, Tim Armstrong,

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

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

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

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
---
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(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/9381/6
--
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: newpatchset
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 


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

2018-03-06 Thread Tim Armstrong (Code Review)
Hello Quanlong Huang, Lars Volker, Laszlo Gaal, Jim Apple,

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

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

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

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

IMPALA-5717: Build ORC C++ lib in toolchain

This adds the C++ library from the Apache ORC project. 1.4.3 is the
latest release at the time of writing.

Includes two patches:
* One that allows building ORC against our versions of dependencies.
  ORC-266 fixes this upstream, but could not be cleanly cherry-picked.
* One that fixes a header that wasn't installed properly.

Testing:
Confirmed that it built locally, confirmed that the built utilities were
runnable after sourcing impala-config.sh.

Built on all supported distros.

Change-Id: I6abd86a5a683f19aa44b47629edf283b938b7e7e
---
M buildall.sh
A source/orc/build.sh
A 
source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch
A 
source/orc/orc-1.4.3-patches/0002-ORC-239.-Install-missing-Statistics.hh-header-file.patch
4 files changed, 224 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/74/9274/5
--
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: newpatchset
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 


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

2018-03-06 Thread 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 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9274/4/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/9274/4/buildall.sh@320
PS4, Line 320:   export LZ4_VERSION=1.7.5
> Remind me what happens if these end up inconsistent with other _VERSIONs fr
Building from scratch will fail since the directories won't exist. I feel like 
this mechanism is a bit clunky but it's consistent with what we're already 
doing.


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh
File source/orc/build.sh:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh@38
PS4, Line 38: DBUILD_LZ4
> Even though compression BUILDs are OFF, we still need to specify what versi
It seems to be sufficient to point it at the header and library directories. 
I'll revisit once we pick up the ORC-266 patch to see what they did there.


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch
File 
source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch@4
PS4, Line 4: PATCH 1/2
> What is part 2/2? Also, is there a JIRA for this? Is it slated to land upst
This was an artifact of me generating it with "git format-patch -2" on a branch 
with another commit that I replaced with the other patch.

I discovered after writing this patch that ORC-266 had landed upstream. 
Backporting ORC-266 was a nightmare though, because it was dependent on a 
series of previous patches that entangled the CMake file changes with various 
other changes. So I just decided to stick with this solution. I updated the 
commit message in this patch to make it clear that it shouldn't be carried to 
newer versions.



--
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: 4
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: Tue, 06 Mar 2018 18:59:53 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Attila Jeges (Code Review)
Attila Jeges 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 2:

(15 comments)

Thanks! Few more comments:

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

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@15
PS2, Line 15: 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.
> I observed this behaviour during the investigation and seemed a good idea t
Thanks, I was just curious if it was a general cleanup or something related to 
the JIRA proper.


http://gerrit.cloudera.org:8080/#/c/9420/2/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/2/be/src/runtime/io/disk-io-mgr-test.cc@384
PS2, Line 384: ExecuteWriteFailureTest
> The reason I implemented this way is that In case I followed how InvalidWri
Thanks, I think I got it now :)


http://gerrit.cloudera.org:8080/#/c/9420/4/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/4/be/src/runtime/io/disk-io-mgr-test.cc@188
PS4, Line 188:  unique_ptrhttp://gerrit.cloudera.org:8080/#/c/9420/4/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/4/be/src/runtime/io/disk-io-mgr-with-fault-injection.h@53
PS4, Line 53:
const string& ?


http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/disk-io-mgr-with-fault-injection.h@60
PS4, Line 60:
Have you considered adding a public member function to 
DiskIoMgrWithFaultInjection class to set/clear 'fault_injection_to_write_' ?

Using the class like this is a bit awkward.

Also probably DiskIoMgrTest would not have to be a friend class then.


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

http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/disk-io-mgr.h@402
PS4, Line 402: st in
nit: returned


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

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@440
PS2, Line 440: Function names can be "open", "fdopen", "fseek", "fwrite"
 :   // and "fclose".
> I was thinking of this as well. The reason I chose strings is that I'd like
If you add a public member function to this class to set/clear 
'faultInjectionToWrite_' (as I suggested), then passing in an enum parameter to 
that function would be more straightforward. You still need to map the enum 
literals to strings.

On the other hand it might be an overkill. Your call ;)


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

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@812
PS2, Line 812: DebugFaultInjection
> This is the point where I set errno to some desired value. Once it is set,
Thanks, my bad. I meant to comment this on GetErrorStatusFromErrno()


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

http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/disk-io-mgr.cc@1107
PS4, Line 1107:
nit: parentheses are unnecessary.


http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/errno-to-error-status-converter.h
File be/src/runtime/io/errno-to-error-status-converter.h:

http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/errno-to-error-status-converter.h@43
PS4, Line 43: ror text r
Maybe this should be called 'errno_' or something similar to avoid confusing it 
with an Impala error code. Also, the comment above should refer to the formal 
parameter name. Please update it.


http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/errno-to-error-status-converter.h@52
PS4, Line 52:  function
Same here


http://gerrit.cloudera.org:8080/#/c/9420/4/be/src/runtime/io/errno-to-error-status-converter.h@56
PS4, Line 56:
Same here


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.cc
File be/src/runtime/io/errno-to-error-status-converter.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.cc@25
PS2, Line 25: unordered_map 
ErrnoToErrorStatusConverter::errnoToErrorTextMap_(
: {{EACCES, "Access denied for the process' user"},
:  {EINTR,   "Internal error occured."},
:  {EINVAL,  "Invalid inputs."},
:  {EMFILE,  "Process level opened file descriptor count is 
reached."},
:  {ENAMETOOLONG,
:  "E

[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
> We already depend on PostgreSQL in development environments because of the
+1. It would be helpful to have infrastructure to make it easy to compare expr 
results against postgres in cases where we expect them to be the same. It might 
also be useful to compare against hive too, although that might be slower.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:47:50 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
> please double check the behavior of DECADE, CENTURY and MILLENNIUM against
We already depend on PostgreSQL in development environments because of the 
minicluster. Should we go full-hog and compare results at test time?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:45:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5607: Add additional units to EXTRACT, DATE PART, TRUNC

2018-03-06 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9391 )

Change subject: IMPALA-5607: Add additional units to EXTRACT, DATE_PART, TRUNC
..


Patch Set 2:

Sure! Thank you.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3df97d685af3e81a40ce8a4f277a5c543d3ea57
Gerrit-Change-Number: 9391
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:40:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

2018-03-06 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..


Patch Set 8:

(2 comments)

Once you address my comments below, please rebase with the master branch as 
that has KRPC enabled by default and make sure that both secure and non-secure 
environments run properly.

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

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@77
PS7, Line 77: r (const auto& prot
> Removed. Thrift added this enum and I was not sure whether it should be her
I just realized. We need to maintain parity with KRPC as well:
https://github.com/apache/kudu/blob/2a5a12fbd69882b387f83e1da71d8a0fd19b6a63/src/kudu/server/server_base.cc#L168-L173

This means that we cannot use the SSLTLS option too. Let's just stick with 
TLSv1_0, TLSv1_1, and TLSv1_2.

In that case, we can leave the 'ssl_minimum_version' flag as it is.


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@99
PS7, Line 99: return true;
> I mean  protocol == SSLTLS || protocol == TLSv1_0 || protocol == TLSv1_1
Yup, this is right, but will now need to change anyway because we'll need to 
get rid of SSLTLS.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:37:00 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9274 )

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


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9274/4/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/9274/4/buildall.sh@320
PS4, Line 320:   export LZ4_VERSION=1.7.5
Remind me what happens if these end up inconsistent with other _VERSIONs from 
other components? It looks to me like they all match now.


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh
File source/orc/build.sh:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh@38
PS4, Line 38: DBUILD_LZ4
Even though compression BUILDs are OFF, we still need to specify what version 
is needed?


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch
File 
source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch@4
PS4, Line 4: PATCH 1/2
What is part 2/2? Also, is there a JIRA for this? Is it slated to land upstream?



--
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: 4
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: Tue, 06 Mar 2018 18:13:35 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread 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:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9500/2/be/src/exec/parquet-common.h@39
PS2, Line 39: vlaidate
Typo.



--
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-Comment-Date: Tue, 06 Mar 2018 17:59:42 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#2). ( 
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
---
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/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: newpatchset
Gerrit-Change-Id: I1502ea7b7f39aa09f0ed2677e84219b37c64c416
Gerrit-Change-Number: 9500
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 


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

2018-03-06 Thread Adam Holley (Code Review)
Adam Holley has uploaded this change for review. ( 
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/Authorizeable.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
6 files changed, 57 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/9509/1
--
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: newchange
Gerrit-Change-Id: I9676a21a171862bb1f664e82d0a7d1db0d36462a
Gerrit-Change-Number: 9509
Gerrit-PatchSet: 1
Gerrit-Owner: Adam Holley 


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

2018-03-06 Thread 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 4:

Just getting back to this. I know a lot of people at Cloudera have been busy 
the last couple of weeks between various events and deadlines.


--
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: 4
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: Tue, 06 Mar 2018 17:33:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9508 )

Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/expr-test.cc@8351
PS1, Line 8351:   TimestampValue::Parse("2110-01-01 00:00:00"));
please double check the behavior of DECADE, CENTURY and MILLENNIUM against 
postgres, if you haven't already, just to be extra sure we get it right this 
time around.


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc
File be/src/exprs/udf-builtins.cc:

http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@144
PS1, Line 144: n
missing an 'n' in spelling of Millennium


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@145
PS1, Line 145:   unsigned short y = orig_date.year();
maybe add a quick comment explaining this, from your commit message:
// First year of current millennium is 2001.


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@150
PS1, Line 150:   }
I think this handles both cases?

y = (y - 1) / 1000 * 1000 + 1


http://gerrit.cloudera.org:8080/#/c/9508/1/be/src/exprs/udf-builtins.cc@163
PS1, Line 163:   }
same comments



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 17:32:36 +
Gerrit-HasComments: Yes


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

2018-03-06 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy 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 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9381/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9381/5//COMMIT_MSG@12
PS5, Line 12: ignores
> If two NaN's are inserted, it wouldn't ignore them but write them into the
Yeah, the next sentence describes that case. Let me rephrase it.

I'm not sure if this is the best option, but it was intentional. I had a 
discussion with the parquet-cpp community about the behavior of the quick fix. 
We agreed to follow what fmax/fmin does: they only return NaN when both 
arguments are NaNs.

IMPALA-6539 is about the long-term fix, but it can't be implemented until 
PARQUET-1222 is resolved.


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

http://gerrit.cloudera.org:8080/#/c/9381/5/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@580
PS5, Line 580: 
> Can you add a test that inserts multiple NaNs, with and without following t
Added some tests for these cases.



--
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: 5
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: Tue, 06 Mar 2018 17:25:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6606: date trunc() misinterprets MILLENNIUM/CENTURY precision

2018-03-06 Thread Attila Jeges (Code Review)
Attila Jeges has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9508


Change subject: IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY 
precision
..

IMPALA-6606: date_trunc() misinterprets MILLENNIUM/CENTURY precision

This change fixes the following correctness bug: date_trunc() function
returns the wrong result when truncating a TIMESTAMP value to
'MILLENNIUM' precision. E.g.:

select now(), date_trunc('millennium', now());
+---+-+
| now() | date_trunc('millennium', now()) |
+---+-+
| 2017-12-05 14:00:30.296812000 | 2000-01-01 00:00:00 |
+---+-+

The first year of the current millennium should be 2001.

'CENTURY' precision has the same issue.

Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
---
M be/src/exprs/expr-test.cc
M be/src/exprs/udf-builtins.cc
2 files changed, 48 insertions(+), 22 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f4aac17875dbf17dcabff25473ffeb006fce20b
Gerrit-Change-Number: 9508
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 


[Impala-ASF-CR] IMPALA-5607: Add additional units to EXTRACT, DATE PART, TRUNC

2018-03-06 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9391 )

Change subject: IMPALA-5607: Add additional units to EXTRACT, DATE_PART, TRUNC
..


Patch Set 2:

Hey,
Let me put myself in context and get back to you with my observations in 1-2 
days of timeframe? Would that be fine?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3df97d685af3e81a40ce8a4f277a5c543d3ea57
Gerrit-Change-Number: 9391
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Comment-Date: Tue, 06 Mar 2018 16:34:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

2018-03-06 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9403/2/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/9403/2/tests/query_test/test_scanners.py@649
PS2, Line 649: "testdata/data/", file_name)
> I have copy-pasted the template of this functions from test_def_levels, whi
For me a refactor you mentioned makes sense. Do you know the proper way of 
doing this? Open a new Jira, or proactively include this to a random patch that 
touches this area (like this patch)?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 16:15:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..

IMPALA-6595: fix crash in NljBuilder::Close()

The bug is that the right child of a blocking join node could be closed
before the builder if an error was encountered when sending a batch to
the sink. This hits a DCHECK because Buffers owned by the sink may still
be accounted against the child node.

Testing:
Added the test that originally triggered the problem. It reproduced the
failure when based on the IMPALA-4835 patch, but I can't reproduce
the failure after rebase onto master.

Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Reviewed-on: http://gerrit.cloudera.org:8080/9493
Tested-by: Impala Public Jenkins
Reviewed-by: Tim Armstrong 
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/data-sink.h
M be/src/exec/exec-node.h
M be/src/exec/nested-loop-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
5 files changed, 24 insertions(+), 6 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

2018-03-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 16:12:01 +
Gerrit-HasComments: No


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

2018-03-06 Thread 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 4:

(31 comments)

Thanks for taking a look, Attila and Tim!

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

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@9
PS2, Line 9: enhanced
> typo: enhanced
Done


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@11
PS2, Line 11: function
> typo: functions
Done


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@12
PS2, Line 12: If a
> If
Done


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@13
PS2, Line 13: then
> typo: then
Done


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@20
PS2, Line 20: function
> typo: functions
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/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/2/be/src/runtime/io/disk-io-mgr-test.cc@242
PS2, Line 242:
> Probably you should move L242-L255 after L236
Rebase solved this :)


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@242
PS2, Line 242:  _1)
> making
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@245
PS2, Line 245:(*new_range)->SetData(reint
> 'phase_to_fail' specifies the name
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@246
PS2, Line 246: PECT_OK(
> "fdopen"
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@250
PS2, Line 250:
> ExecuteWriteFailureTest()
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@376
PS2, Line 376:
> The name is misleading as errors in functions other than fdopen() are also
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@384
PS2, Line 384: write_range, nu
> Mayb it woyld be cleaner if you passed "/tmp/disk_io_mgr_test.txt" to Execu
Made "/tmp/disk_io_mgr_test.txt" a parameter, also removed it being hard-coded 
from the error text.


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@418
PS2, Line 418:
> tmp_file would probably work too?
Done


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

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@443
PS2, Line 443:
> nit: fault_injection_to_write_
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@443
PS2, Line 443:
> It might be more readable if we define a struct here.
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@445
PS2, Line 445: disk_thread_group_;
> 'phase' parameter
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@448
PS2, Line 448: ached_
> const string& ?
Done


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

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@765
PS2, Line 765: disk_id = disk_queue->disk_id;
> Seems a good idea not to mess the prod code for fault injection. I'll give
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@782
PS2, Line 782: // Get the next reader and remove the reader so that another disk
> Shouldn't this be:
Indeed, thx.
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@812
PS2, Line 812:
> const string& ?
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@812
PS2, Line 812:
> It would be safer to pass in 'errno' as a parameter. Remember that any syst
This is the point where I set errno to some desired value. Once it is set, not 
much happening until GetErrorStatusFromErrno() is called. I modified that 
function to receive errno as param.

Is this fine or should I think of any other issues?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.h
File be/src/runtime/io/errno-to-error-status-converter.h:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.h@28
PS2, Line 28: /// This class translates 'errno' values set by disk I/O related 
functions to Status
> Avoid "using" declarations in headers.
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.h@31
PS2, Line 31:
> Maybe something like this instead :
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.h@41
PS2, Line 41: TextM
> set
Done


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.h@43
PS2, Line 43: e, const Params& param
> corresponding to 'errno'
Done


http://gerrit.cloudera.org:8

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

2018-03-06 Thread 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 (#4).

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, 449 insertions(+), 76 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/9420/4
--
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: 4
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

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

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 10:15:27 +
Gerrit-HasComments: No


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

2018-03-06 Thread Donghui Xu (Code Review)
Donghui Xu has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9506


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_cmd Impala shell get contains extra line break.

I fix this issue by removing extraneous '\n' in value when impala-shell using
ldap_password_cmd.

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/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: newchange
Gerrit-Change-Id: Ie570166aea62af223905b7f0124e9efb15a88ac7
Gerrit-Change-Number: 9506
Gerrit-PatchSet: 1
Gerrit-Owner: Donghui Xu 


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

2018-03-06 Thread 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 3:

Rebased with master + resolved conflicts.


-- 
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: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 08:10:31 +
Gerrit-HasComments: No


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

2018-03-06 Thread 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 (#3).

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 anhanced by this
change. A mapping is introduced between the errno set by open(),
fdopen(), fseek(), fwrite() or fclose() low level function and an
error message for displaying purposes. Once any of these functions
fail than 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 funtions fail.

Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7
---
M be/src/runtime/io/CMakeLists.txt
M be/src/runtime/io/disk-io-mgr-test.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
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
10 files changed, 322 insertions(+), 75 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/9420/3
--
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: 3
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong