date:20160929

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4118: extract encryption utils from BufferedBlockMgr
..


Patch Set 10: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4389/10/be/src/runtime/buffered-block-mgr.cc
File be/src/runtime/buffered-block-mgr.cc:

Line 1342:   // the block and access the buffer concurrently.
Are blocks really shared across threads like that?  Anyway, fine to leave this 
but let's not bring this logic over to buffer-pool unless necessary.


http://gerrit.cloudera.org:8080/#/c/4389/10/be/src/runtime/buffered-block-mgr.h
File be/src/runtime/buffered-block-mgr.h:

PS10, Line 255: having no
  : /// references to the data
I didn't know what this was trying to say until I saw the clearer comment in 
the .cc file.  How about something like
The read path can be decrypted in place since no one else could have a 
reference to the read buffer.


PS10, Line 656: temporary
is this talking about encrypted_write_buffer_, or something else?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibebe431f69c3c569cbff68171beaa32ef2ab03b6
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-889: Add support for an ISO-SQL compliant trim() function.

2016-09-29 Thread Youwei Wang (Code Review)

Youwei Wang has uploaded a new patch set (#5).

Change subject: IMPALA-889: Add support for an ISO-SQL compliant trim() 
function.
..

IMPALA-889: Add support for an ISO-SQL compliant trim() function.

Purpose: Removes all instances of one or more characters from the
  specified direction(s) of a STRING value.
Syntax #1 TRIM(where FROM string_to_be_trimmed);
Syntax #2 TRIM(trim_char_set FROM string_to_be_trimmed);
Syntax #3 TRIM(where trim_char_set FROM string_to_be_trimmed);
Syntax #3 confirms the standard SQL syntax (Core SQL feature ID
  E021-09).

"where": Case-insensitive trim direction. Valid options are:
  leading|trailing|both. leading means trimming characters from the
  start; trailing means trimming characters from the end; both means
  trimming characters from both sides. These options should be
  provided without single/double quotation marks since they are not
  string literals but identifiers. NULL or empty argument implies
  "both" option. If an invalid option is specified, returns target
  untouched.
"trim_char_set": Case-sensitive characters to be removed. This
  argument is regarded as a character set going to be removed. So the
  occurrence order of each character doesn't matter and duplicated
  instance of same character will be ignored. NULL argument implies
  " "(space) by default. Empty argument return string_to_be_trimmed
  untouched.
"target": Case-sensitive target string to trim. This argument can be
  NULL. Blank argument causes parsing error.
Note: For Syntax #1, since no "characters" is specified, it trims
" "(space) by default. For Syntax #2, since no "where" is specified,
the option both is implied by default.
Return type: string

Examples:
Syntax #1:
trim(both 'abfg%' from 'abc%%defg%'); returns 'c%%de';
trim(leading FROM ' 123 '); returns '123 ';
trim(trailing FROM ' 123 '); returns ' 123';
Syntax #2:
trim('&@' FROM '&@&@&@&127+1-@&@&@&@')"; returns "127+1-";
Syntax #3:
trim(leading 'ab%' from 'abc%%defg%'); returns 'c%%defg%';
trim(trailing 'bfg%' from 'abc%%defg%'); returns 'abc%%de';
trim(both 'abfg%' from 'abc%%defg%')"; returns "c%%de";

Change-Id: I4753c608b0b00569bf8c5e95b132df6df358e602
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
M common/thrift/Exprs.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/com/cloudera/impala/analysis/TrimExpr.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java
9 files changed, 429 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4753c608b0b00569bf8c5e95b132df6df358e602
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-889: Add support for an ISO-SQL compliant trim() function.

2016-09-29 Thread Youwei Wang (Code Review)

Youwei Wang has uploaded a new patch set (#4).

Change subject: IMPALA-889: Add support for an ISO-SQL compliant trim() 
function.
..

IMPALA-889: Add support for an ISO-SQL compliant trim() function.

Purpose: Removes all instances of one or more characters from the
  specified direction(s) of a STRING value.
Syntax #1 TRIM(where FROM string_to_be_trimmed);
Syntax #2 TRIM(trim_char_set FROM string_to_be_trimmed);
Syntax #3 TRIM(where trim_char_set FROM string_to_be_trimmed);
Syntax #3 confirms the standard SQL syntax (Core SQL feature ID
  E021-09).

"where": Case-insensitive trim direction. Valid options are:
  leading|trailing|both. leading means trimming characters from the
  start; trailing means trimming characters from the end; both means
  trimming characters from both sides. These options should be
  provided without single/double quotation marks since they are not
  string literals but identifiers. NULL or empty argument implies
  "both" option. If an invalid option is specified, returns target
  untouched.
"trim_char_set": Case-sensitive characters to be removed. This
  argument is regarded as a character set going to be removed. So the
  occurrence order of each character doesn't matter and duplicated
  instance of same character will be ignored. NULL argument implies
  " "(space) by default. Empty argument return string_to_be_trimmed
  untouched.
"target": Case-sensitive target string to trim. This argument can be
  NULL. Blank argument causes parsing error.
Note: For Syntax #1, since no "characters" is specified, it trims
" "(space) by default. For Syntax #2, since no "where" is specified,
the option both is implied by default.
Return type: string

Examples:
Syntax #1:
trim(both 'abfg%' from 'abc%%defg%'); returns 'c%%de';
trim(leading FROM ' 123 '); returns '123 ';
trim(trailing FROM ' 123 '); returns ' 123';
Syntax #2:
trim('&@' FROM '&@&@&@&127+1-@&@&@&@')"; returns "127+1-";
Syntax #3:
trim(leading 'ab%' from 'abc%%defg%'); returns 'c%%defg%';
trim(trailing 'bfg%' from 'abc%%defg%'); returns 'abc%%de';
trim(both 'abfg%' from 'abc%%defg%')"; returns "c%%de";

Change-Id: I4753c608b0b00569bf8c5e95b132df6df358e602
---
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M common/function-registry/impala_functions.py
A common/function-registry/string-functions-ir.cc
M common/thrift/Exprs.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/com/cloudera/impala/analysis/TrimExpr.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeExprsTest.java
10 files changed, 1,439 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4753c608b0b00569bf8c5e95b132df6df358e602
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Youwei Wang

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Michael Ho has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..


Patch Set 12: Code-Review+2

Carry +2 forward.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Michael Ho has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4350/11/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:

PS11, Line 188: ADD_COUNTER
> nit: ADD_TIMER
Done


http://gerrit.cloudera.org:8080/#/c/4350/11/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

Line 190: // The size of 'get_list_' is read racily without holding 
'get_lock_'.
> .. to avoid taking the get_lock_ on the Put path.
Done


PS11, Line 208: callers
> remove the word "callers".  these functions wait on it directly.
Done


PS11, Line 228: callers
> remove "callers".
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Hello Dan Hecht, Tim Armstrong,

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

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

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

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..

IMPALA-4026: Implement double-buffering for BlockingQueue

With recent changes to improve the parquet scanner's efficency,
row batches are produced more quickly, leading to higher contention
in the blocking queue shared between scanner threads and the scan
node. The contention happens between different producers (i.e.
the scanner threads) and also to a lesser extent, between the
scanner threads and the scan node.

This change addresses the contention between the scanner threads
and the scan node by splitting the queue into a 'get_list_' and
a 'put_list_'. The consumers will consume from 'get_list_' until
it's exhausted while the producers will enqueue into 'put_list_'
until it's full. When 'get_list_' is exhausted, the consumer will
atomically swap the 'get_list_' with 'put_list_'. This reduces
the contention: 'get_list_' and 'put_list_' are protected by two
different locks so callers of BlockingGet() only contends for the
'put_lock_' when 'put_list_' is empty.

With this change, primitive_filter_bigint_non_selective improves
by 33.9%, going from 1.60s to 1.06s

Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
---
M be/src/common/compiler-util.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/util/blocking-queue.h
A be/src/util/condition-variable.h
M be/src/util/thread-pool.h
6 files changed, 220 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/4350/12
-- 
To view, visit http://gerrit.cloudera.org:8080/4350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_

HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.

Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Reviewed-on: http://gerrit.cloudera.org:8080/4558
Reviewed-by: Dan Hecht 
Tested-by: Internal Jenkins
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
5 files changed, 39 insertions(+), 7 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Dan Hecht: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4135: Thrift threaded server times-out connections during high load

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4135: Thrift threaded server times-out connections 
during high load
..


Patch Set 4:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:

PS3, Line 204: 
> this is an important enough parameter that I'd make a constant for it, and 
Done


PS3, Line 216: 
> If this returns false it's because the queue is shut down. Better to say th
Done


PS3, Line 218: break;
> set stop_ to true to cleanup?
Done


http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/TAcceptQueueServer.h
File be/src/rpc/TAcceptQueueServer.h:

Line 46:  *
> add a reference to IMPALA-4135 here.
Done


PS3, Line 105: 
> Is this used anywhere?
Done


http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/thrift-server-test.cc
File be/src/rpc/thrift-server-test.cc:

Line 174:   // Test that a large number of concurrent connections will all 
succeed and not time out
> Add a comment about what you're testing and the associated JIRA.
Done


PS3, Line 178: _OK(server->Start(
> Does this work in a threaded context - does the test fail if the thread hit
Yes, although the test runs to completion before reporting the error.


PS3, Line 180: ThreadPool pool(
> I'm a bit concerned about failing with ulimit errors, particularly because 
Agreed, this test has a lot of potential for flakiness or other pain. Not sure 
how else to do an automatic test of this issue, though.

As written, it doesn't even actually fail on my machine without the fix, unless 
I add an artificial slowdown.


PS3, Line 173: TEST(ConcurrencyTest, ConnectTimeout) {
 :   // Test that a large number of concurrent connections will all 
succeed and not time out
 :   // waiting to be accepted.(IMPALA-4135)
 :   int port = GetServerPort();
 :   ThriftServer* server = new ThriftServer("DummyServer", 
MakeProcessor(), port);
 :   ASSERT_OK(server->Start());
 : 
 :   ThreadPool pool(
 :   "group", "test", 256, 1, [port](int tid, const 
int64_t& item) {
 :  
> This relies on a running impala internal service - you probably had an Impa
Done


http://gerrit.cloudera.org:8080/#/c/4519/1/be/src/server/TAcceptQueueThreadedServer.cpp
File be/src/server/TAcceptQueueThreadedServer.cpp:

PS1, Line 233: 
> Oops, sorry about that.
No problem.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4135: Thrift threaded server times-out connections during high load

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has uploaded a new patch set (#4).

Change subject: IMPALA-4135: Thrift threaded server times-out connections 
during high load
..

IMPALA-4135: Thrift threaded server times-out connections during high load

During times of high load, Thrift's TThreadedServer can't keep up with the
rate of new socket connections, causing some to time out.

This patch creates TAcceptQueueServer, which is a modified version of
TThreadedServer that calls accept() and then hands the returned TTransport
off to a thread pool to handle setting up the connection. This ensures that
accept() is called as quickly as possible, preventing connections from timing
out while waiting.

This patch has been tested locally with the repro shown in IMPALA-4135, but
it still needs to be tested in a real cluster.

Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
---
M be/src/common/global-flags.cc
M be/src/rpc/CMakeLists.txt
A be/src/rpc/TAcceptQueueServer.cpp
A be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
6 files changed, 457 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] Remove spurious Boost warnings on compilation errors

Internal Jenkins has posted comments on this change.

Change subject: Remove spurious Boost warnings on compilation errors
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib84d8a9958469fb22b0af4907958917a65e8290f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] Remove spurious Boost warnings on compilation errors

Internal Jenkins has submitted this change and it was merged.

Change subject: Remove spurious Boost warnings on compilation errors
..


Remove spurious Boost warnings on compilation errors

Compilation errors can spuriously print warnings from Boost where the
filesystem module is used, like:

‘boost::system::posix_category’ defined but not used

Defining BOOST_SYSTEM_NO_DEPRECATED removes those warnings, which arise
from Boost maintaining deprecated names for error codes that have moved
namespaces during the shift to C++11 (see
http://www.boost.org/doc/libs/1_61_0/boost/system/error_code.hpp and
http://www.boost.org/doc/libs/1_61_0/libs/system/doc/reference.html).

We're not using the old names, so it's ok to remove them.

Change-Id: Ib84d8a9958469fb22b0af4907958917a65e8290f
Reviewed-on: http://gerrit.cloudera.org:8080/4564
Reviewed-by: Tim Armstrong 
Tested-by: Internal Jenkins
---
M be/CMakeLists.txt
1 file changed, 1 insertion(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib84d8a9958469fb22b0af4907958917a65e8290f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

2016-09-29 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 3:

(1 comment)

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

Line 2499: select count(shiftleft(int_col, 1)) from functional_parquet.alltypes
> before this fix, did the query fail, or did it just give warnings?
It fails with a warning. 

Builtin 'shiftleft' with symbol 
'_ZN6impala16BitByteFunctions9ShiftLeftEPN10impala_udf15FunctionContextERKNS1_6IntValES6_'
 does not exist. Verify that all your impalads are the same version.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4135: Thrift threaded server times-out connections during high load

2016-09-29 Thread Juan Yu (Code Review)

Juan Yu has posted comments on this change.

Change subject: IMPALA-4135: Thrift threaded server times-out connections 
during high load
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:

PS3, Line 218: 
set stop_ to true to cleanup?


http://gerrit.cloudera.org:8080/#/c/4519/1/be/src/server/TAcceptQueueThreadedServer.cpp
File be/src/server/TAcceptQueueThreadedServer.cpp:

PS1, Line 233: if (stop_) {
> Its not in the while loop.
Oops, sorry about that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4023: don't attach buffered tuple streams to batches

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4023: don't attach buffered tuple streams to batches
..


Patch Set 4: Verified-1

Build failed: 
http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/252/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..


Patch Set 11: Code-Review+2

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4350/11/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:

PS11, Line 188: ADD_COUNTER
nit: ADD_TIMER


http://gerrit.cloudera.org:8080/#/c/4350/11/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

Line 190: // The size of 'get_list_' is read racily without holding 
'get_lock_'.
.. to avoid taking the get_lock_ on the Put path.
(to explain the "why" not just the "what").


PS11, Line 208: callers
remove the word "callers".  these functions wait on it directly.


PS11, Line 228: callers
remove "callers".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Michael Ho has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 3:

Taras also verified the fix works with the same query.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..


IMPALA-3786: Replace "cloudera" with "apache" (part 2)

As part of the ASF transition, we need to replace references to
Cloudera in Impala with references to Apache. This primarily means
changing Java package names from com.cloudera.impala.* to
org.apache.impala.*

A prior patch renamed all the files as necessary, and this patch
performs the actual code changes. Most of the changes in this patch
were generated with some commands of the form:

find . | grep "\.java\|\.py\|\.h\|\.cc" | \
  xargs sed -i s/'com\(.\)cloudera\(\.\)impala/org\1apache\2impala/g

along with some manual fixes.

After this patch, the remaining references to Cloudera in the repo
mostly fall into the categories:
- External components that have cloudera in their own package names,
  eg. com.cloudera.kudu/llama
- URLs, eg. https://repository.cloudera.com/

Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
Reviewed-on: http://gerrit.cloudera.org:8080/3937
Reviewed-by: Thomas Tauber-Marshall 
Reviewed-by: Jim Apple 
Tested-by: Internal Jenkins
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/catalog/catalog.cc
M be/src/catalog/catalog.h
M be/src/exec/external-data-source-executor.cc
M be/src/exec/external-data-source-executor.h
M be/src/exprs/hive-udf-call.cc
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/service/fe-support.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/util/jni-util.cc
M be/src/util/logging-support.cc
M bin/create_testdata.sh
M bin/run-jdbc-client.sh
M common/function-registry/CMakeLists.txt
M common/function-registry/gen_builtins_catalog.py
M common/thrift/CatalogInternalService.thrift
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/Data.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Descriptors.thrift
M common/thrift/ExecStats.thrift
M common/thrift/Exprs.thrift
M common/thrift/ExternalDataSource.thrift
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/LineageGraph.thrift
M common/thrift/Logging.thrift
M common/thrift/Metrics.thrift
M common/thrift/Partitions.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Planner.thrift
M common/thrift/Results.thrift
M common/thrift/RuntimeProfile.thrift
M common/thrift/StatestoreService.thrift
M common/thrift/Status.thrift
M common/thrift/Types.thrift
M common/thrift/generate_error_codes.py
M common/thrift/generate_metrics.py
M ext-data-source/api/pom.xml
M 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/util/SerializationUtils.java
M 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/v1/ExternalDataSource.java
M ext-data-source/pom.xml
M ext-data-source/sample/pom.xml
M 
ext-data-source/sample/src/main/java/org/apache/impala/extdatasource/sample/EchoDataSource.java
M ext-data-source/test/pom.xml
M 
ext-data-source/test/src/main/java/org/apache/impala/extdatasource/AllTypesDataSource.java
M fe/pom.xml
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableChangeColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
M 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1)
..


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1)
..


IMPALA-3786: Replace "cloudera" with "apache" (part 1)

As part of the ASF transition, we need to replace references to
Cloudera in Impala with references to Apache. This primarily means
changing Java package names from com.cloudera.impala.* to
org.apache.impala.*

To make this easier to review, this patch only renames files,
eg. fe/src/main/java/com/cloudera -> fe/src/main/java/org/apache

A follow up patch performs the actual code updates.

Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db
Reviewed-on: http://gerrit.cloudera.org:8080/3936
Reviewed-by: Thomas Tauber-Marshall 
Reviewed-by: Jim Apple 
Tested-by: Internal Jenkins
---
R 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/util/SerializationUtils.java
R 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/v1/ExternalDataSource.java
R 
ext-data-source/sample/src/main/java/org/apache/impala/extdatasource/sample/EchoDataSource.java
R 
ext-data-source/test/src/main/java/org/apache/impala/extdatasource/AllTypesDataSource.java
R fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
R fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableChangeColStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
R 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
R fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
R fe/src/main/java/org/apache/impala/analysis/Analyzer.java
R fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
R fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
R fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
R fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
R fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
R fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
R fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
R fe/src/main/java/org/apache/impala/analysis/CaseWhenClause.java
R fe/src/main/java/org/apache/impala/analysis/CastExpr.java
R fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java
R fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
R fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
R fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java
R fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
R fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
R fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateUdaStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
R fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
R fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
R fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
R

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 3:

> (1 comment)

Taras, were you also able to verify that the fix solves the crash?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Michael Ho has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4558/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:

Line 166: select * from t t1 left join t t2 on t1.x > 0
> did you verify that this query reproduces a crash?
Confirmed by Taras. Thanks Taras !


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

Michael Ho has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4023: don't attach buffered tuple streams to batches

Hello Internal Jenkins, Alex Behm, Dan Hecht,

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

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

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

Change subject: IMPALA-4023: don't attach buffered tuple streams to batches
..

IMPALA-4023: don't attach buffered tuple streams to batches

This simplifies the memory transfer model by eliminating one category of
resources that can be attached.

Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
---
M be/src/exec/analytic-eval-node.cc
M be/src/exec/analytic-eval-node.h
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
10 files changed, 110 insertions(+), 96 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4023: don't attach buffered tuple streams to batches

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4023: don't attach buffered tuple streams to batches
..


Patch Set 4: Code-Review+2

Missed updating a DCHECK for the nested types codepath.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6471422f86ce71e4c6ab277a27651bc2e8ff
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

2016-09-29 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 3: Code-Review+1

(1 comment)

Carry +1

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

PS2, Line 2498: built-i
> built-in ?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is 
set
..


Patch Set 1:

I tested this when AUX is present; it passed. I'm now testing when AUX is 
absent.

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

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

[Impala-ASF-CR] IMPALA-784: Use `-s in SHOW CREATE TABLE output

2016-09-29 Thread Zoltan Ivanfi (Code Review)

Zoltan Ivanfi has posted comments on this change.

Change subject: IMPALA-784: Use `-s in SHOW CREATE TABLE output
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4527/2/fe/src/main/java/com/cloudera/impala/analysis/ToSqlUtils.java
File fe/src/main/java/com/cloudera/impala/analysis/ToSqlUtils.java:

Line 76: return 
Joiner.on(".").join(getStandaloneIdentSqlList((Splitter.on('.').split(ident;
> Those are two separate issues. In your example, we are guarding against a l
I added splitting because when I just put the whole string between backticks 
then Impala ended up escaping qualified identifiers of complex types 
incorrectly. I noticed that these references were already in a string form by 
the time getIdentSql gets called, so there is no way to properly quote them 
without refactoring to pass them in a structured format to the affected 
functions.

The "Invalid column/field name" error message mislead me to believe that dots 
are not allowed in any identifier, which lead me to come up with my approach 
which would work if identifiers really couldn't contain dots. Since this turned 
out to be a false assumption, it seems that I have to discard this change as 
this task can only be implemented properly after IMPALA-2287 and probably some 
other refactorings.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e66439daa35d2e3725e7277c16f3cc5fdb6f128
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove spurious Boost warnings on compilation errors

Tim Armstrong has posted comments on this change.

Change subject: Remove spurious Boost warnings on compilation errors
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib84d8a9958469fb22b0af4907958917a65e8290f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4558/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:

Line 166: select * from t t1 left join t t2 on t1.x > 0
did you verify that this query reproduces a crash?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove spurious Boost warnings on compilation errors

2016-09-29 Thread Henry Robinson (Code Review)

Henry Robinson has uploaded a new change for review.

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

Change subject: Remove spurious Boost warnings on compilation errors
..

Remove spurious Boost warnings on compilation errors

Compilation errors can spuriously print warnings from Boost where the
filesystem module is used, like:

‘boost::system::posix_category’ defined but not used

Defining BOOST_SYSTEM_NO_DEPRECATED removes those warnings, which arise
from Boost maintaining deprecated names for error codes that have moved
namespaces during the shift to C++11 (see
http://www.boost.org/doc/libs/1_61_0/boost/system/error_code.hpp and
http://www.boost.org/doc/libs/1_61_0/libs/system/doc/reference.html).

We're not using the old names, so it's ok to remove them.

Change-Id: Ib84d8a9958469fb22b0af4907958917a65e8290f
---
M be/CMakeLists.txt
1 file changed, 1 insertion(+), 0 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

Michael Ho has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 2:

(1 comment)

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

PS2, Line 2498: inbuilt
built-in ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Michael Ho has uploaded a new patch set (#3).

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..

IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_

HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.

Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
5 files changed, 39 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Michael Ho has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 302:   for (DiskIoRequestContext* context: reader_contexts_) {
> Nit clang-format puts a space before :, should do that or consistency.
Done


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

Line 165:   /// Takes ownership of a scan node's reader context and unregisters 
it when the
> "Takes ownership of the given reader context which may still hold used IO b
It's "automatic" from the perspective of the caller but I know what you mean.


Line 167:   /// TODO: Attach the reader context to the last row batch instead.
> Works for me. If the eventual direction is not clear, then let's remove the
Sure. Let's remove it for now but that sounds like an attractive option if 
unregistration happens at the end of the lifetime of the IO buffer.


http://gerrit.cloudera.org:8080/#/c/4558/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:

Line 215:  QUERY
> move this below the test for IMPALA-561 to cluster related tests together
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

Jim Apple has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..


Patch Set 6: Code-Review+2

Carry ALex's +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)

Jim Apple has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1)
..


Patch Set 5: Code-Review+2

Carry ALex's +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..


Patch Set 6: Code-Review+1

Rebased, carrying forward +1s.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3937/5/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

Line 90:   "org.apache.impala.hive.serde.ParquetInputFormat",
> We cannot change this because it might break existing tables. Please revert
Done


http://gerrit.cloudera.org:8080/#/c/3937/5/fe/src/test/java/org/apache/impala/catalog/HdfsStorageDescriptorTest.java
File fe/src/test/java/org/apache/impala/catalog/HdfsStorageDescriptorTest.java:

Line 58: "org.apache.impala.hive.serde.ParquetInputFormat",
> Please revert (see comment in HdfsFileFormat.java)
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is 
set
..


Patch Set 1:

Testing now. I expect the tests to take a few hours.

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

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

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1)
..


Patch Set 5: Code-Review+1

Rebased, carrying forward +1s.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 2)

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Hello Jim Apple, Alex Behm, Dan Hecht,

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

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

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

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 2)
..

IMPALA-3786: Replace "cloudera" with "apache" (part 2)

As part of the ASF transition, we need to replace references to
Cloudera in Impala with references to Apache. This primarily means
changing Java package names from com.cloudera.impala.* to
org.apache.impala.*

A prior patch renamed all the files as necessary, and this patch
performs the actual code changes. Most of the changes in this patch
were generated with some commands of the form:

find . | grep "\.java\|\.py\|\.h\|\.cc" | \
  xargs sed -i s/'com\(.\)cloudera\(\.\)impala/org\1apache\2impala/g

along with some manual fixes.

After this patch, the remaining references to Cloudera in the repo
mostly fall into the categories:
- External components that have cloudera in their own package names,
  eg. com.cloudera.kudu/llama
- URLs, eg. https://repository.cloudera.com/

Change-Id: I0d35fa6602a7fc0c212b2ef5e2b3322b77dde7e2
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/catalog/catalog.cc
M be/src/catalog/catalog.h
M be/src/exec/external-data-source-executor.cc
M be/src/exec/external-data-source-executor.h
M be/src/exprs/hive-udf-call.cc
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/service/fe-support.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/util/jni-util.cc
M be/src/util/logging-support.cc
M bin/create_testdata.sh
M bin/run-jdbc-client.sh
M common/function-registry/CMakeLists.txt
M common/function-registry/gen_builtins_catalog.py
M common/thrift/CatalogInternalService.thrift
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/Data.thrift
M common/thrift/DataSinks.thrift
M common/thrift/Descriptors.thrift
M common/thrift/ExecStats.thrift
M common/thrift/Exprs.thrift
M common/thrift/ExternalDataSource.thrift
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/LineageGraph.thrift
M common/thrift/Logging.thrift
M common/thrift/Metrics.thrift
M common/thrift/Partitions.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Planner.thrift
M common/thrift/Results.thrift
M common/thrift/RuntimeProfile.thrift
M common/thrift/StatestoreService.thrift
M common/thrift/Status.thrift
M common/thrift/Types.thrift
M common/thrift/generate_error_codes.py
M common/thrift/generate_metrics.py
M ext-data-source/api/pom.xml
M 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/util/SerializationUtils.java
M 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/v1/ExternalDataSource.java
M ext-data-source/pom.xml
M ext-data-source/sample/pom.xml
M 
ext-data-source/sample/src/main/java/org/apache/impala/extdatasource/sample/EchoDataSource.java
M ext-data-source/test/pom.xml
M 
ext-data-source/test/src/main/java/org/apache/impala/extdatasource/AllTypesDataSource.java
M fe/pom.xml
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableChangeColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
M 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
M

[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)

2016-09-29 Thread Thomas Tauber-Marshall (Code Review)

Hello Jim Apple, Alex Behm, Dan Hecht,

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

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

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

Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1)
..

IMPALA-3786: Replace "cloudera" with "apache" (part 1)

As part of the ASF transition, we need to replace references to
Cloudera in Impala with references to Apache. This primarily means
changing Java package names from com.cloudera.impala.* to
org.apache.impala.*

To make this easier to review, this patch only renames files,
eg. fe/src/main/java/com/cloudera -> fe/src/main/java/org/apache

A follow up patch performs the actual code updates.

Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db
---
R 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/util/SerializationUtils.java
R 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/v1/ExternalDataSource.java
R 
ext-data-source/sample/src/main/java/org/apache/impala/extdatasource/sample/EchoDataSource.java
R 
ext-data-source/test/src/main/java/org/apache/impala/extdatasource/AllTypesDataSource.java
R fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
R fe/src/main/java/org/apache/impala/analysis/AggregateInfoBase.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableAddReplaceColsStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableChangeColStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableDropColStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java
R 
fe/src/main/java/org/apache/impala/analysis/AlterTableRecoverPartitionsStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetLocationStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
R fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
R fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
R fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
R fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
R fe/src/main/java/org/apache/impala/analysis/Analyzer.java
R fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
R fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
R fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
R fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
R fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
R fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
R fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
R fe/src/main/java/org/apache/impala/analysis/CaseWhenClause.java
R fe/src/main/java/org/apache/impala/analysis/CastExpr.java
R fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java
R fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
R fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
R fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java
R fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
R fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
R fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateUdaStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateUdfStmt.java
R fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
R fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
R fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
R fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
R fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
R

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

2016-09-29 Thread Alex Behm (Code Review)

Alex Behm has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 2:

(1 comment)

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

Line 167:   /// TODO: Attach the reader context to the last row batch instead.
> This TODO seems too speculative to me, unclear whether the proposed approac
Works for me. If the eventual direction is not clear, then let's remove the 
TODO.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 302:   for (DiskIoRequestContext* context: reader_contexts_) {
Nit clang-format puts a space before :, should do that or consistency.


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

Line 167:   /// TODO: Attach the reader context to the last row batch instead.
This TODO seems too speculative to me, unclear whether the proposed approach is 
the right one, since it's usually simpler not to attach things to row batches, 
and the current approach seems fine to me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

2016-09-29 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..


Patch Set 2:

Sry missed the test. Added it in PS2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4196: Cross compile bit-byte-functions

2016-09-29 Thread Bharath Vissapragada (Code Review)

Hello Michael Ho,

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

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

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

Change subject: IMPALA-4196: Cross compile bit-byte-functions
..

IMPALA-4196: Cross compile bit-byte-functions

Change-Id: I5a1291bfd202b500405a884e4a62f0ca2447244a
---
M be/src/codegen/impala-ir.cc
M be/src/exprs/CMakeLists.txt
R be/src/exprs/bit-byte-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
4 files changed, 10 insertions(+), 1 deletion(-)


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

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

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

Jim Apple has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

PS4, Line 24: 
> Can we rename this to DOWNLOAD_HADOOP_COMPONENTS instead? Or are they CDH s
They are CDH specific right now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add vim-specific files to .gitignore

Tim Armstrong has posted comments on this change.

Change subject: Add vim-specific files to .gitignore
..


Patch Set 1: Code-Review+2

(2 comments)

Seems reasonable.

http://gerrit.cloudera.org:8080/#/c/4562/1/.gitignore
File .gitignore:

Line 1: *~
Can you move this into the vim section?


Line 3: .*.swp
Can you move this into the vim section?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1abcd8ca0e18178684c916ef6f7d55c25c0814a4
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4135: Thrift threaded server times-out connections during high load

2016-09-29 Thread Henry Robinson (Code Review)

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4135: Thrift threaded server times-out connections 
during high load
..


Patch Set 3:

(8 comments)

Some comments before I head off to Strata. I would run clang-format over the 
new thrift files. That way they'll be more readable to Impala developers, while 
keeping the structure and idioms of the thrift code.

http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:

PS3, Line 204: 1
this is an important enough parameter that I'd make a constant for it, and put 
your comments about only using thread on that. We don't want someone changing 
the number of threads without understanding the thread-safety implications.


PS3, Line 216: acceptPool.Offer returned false.
If this returns false it's because the queue is shut down. Better to say that 
clearly in the error message so users have an idea what this might mean. Also 
you can say that it's unexpected - we never shut down our servers so the queue 
should never be shut down.


http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/TAcceptQueueServer.h
File be/src/rpc/TAcceptQueueServer.h:

Line 46:  * connections will time out while in the OS accept queue.
add a reference to IMPALA-4135 here.


PS3, Line 105: std::mutex big_lock_;
Is this used anywhere?


http://gerrit.cloudera.org:8080/#/c/4519/3/be/src/rpc/thrift-server-test.cc
File be/src/rpc/thrift-server-test.cc:

Line 174:   ThreadPool pool("group", "test", 256, 1, [](int tid, 
const int64_t& item) {
Add a comment about what you're testing and the associated JIRA.


PS3, Line 178: ASSERT_OK(status);
Does this work in a threaded context - does the test fail if the thread hits an 
ASSERT?


PS3, Line 180: for (int i = 0; i < 1024 * 16; ++i) pool.Offer(i);
I'm a bit concerned about failing with ulimit errors, particularly because both 
client and server should be in the same process. Have you ever seen this repro 
with fewer concurrent connections - say 8K?


PS3, Line 173: TEST(ConcurrencyTest, ConnectTimeout) {
 :   ThreadPool pool("group", "test", 256, 1, [](int 
tid, const int64_t& item) {
 : using Client = ThriftClient;
 : Client* client = new Client("127.0.0.1", 22000, "", 
NULL, false);
 : Status status = client->Open();
 : ASSERT_OK(status);
 :   });
 :   for (int i = 0; i < 1024 * 16; ++i) pool.Offer(i);
 :   pool.DrainAndShutdown();
 : }
This relies on a running impala internal service - you probably had an Impala 
process running when you ran this? That won't be true in our builds. Use the 
same server code that all the other tests use to start a server in this process.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50e728974ef31a9d49132a0b3f7cde2a4f3356d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3342, IMPALA-3920: Adding thread counters to obtain thread stats per scanner thread, and to measure time spent in per-fragment-executor Open() and ProcessBuildInputAsync calls

2016-09-29 Thread Lars Volker (Code Review)

Lars Volker has posted comments on this change.

Change subject: IMPALA-3342, IMPALA-3920:  Adding  thread counters to obtain 
thread stats per scanner thread, and to measure time spent in 
per-fragment-executor Open() and ProcessBuildInputAsync calls
..


Patch Set 1:

(13 comments)

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

Nit: You might want to set your git user name, so it shows up in your commits.

git config --global user.name "Anuj Phadke"


Line 11: Initial draft. Will add a detailed commit message later.
I couldn't figure out how this addresses IMPALA-3920. Can you explain in the 
commit message?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/exec/blocking-join-node.h
File be/src/exec/blocking-join-node.h:

Line 68:   RuntimeProfile::ThreadCounters* process_build_input_async_counters_;
Can this be protected?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:

Line 358:   string thread_name = "ScannerThread-" + 
std::to_string(syscall(SYS_gettid))+ "-";
Something similar is done in L348. Why are the names different? Also, I think 
we often prefer to use stringstream<< or Substitute() to build strings.

Maybe even better, pass in the name from the caller, similar to the kudu scan 
node.


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/exec/hdfs-scan-node.h
File be/src/exec/hdfs-scan-node.h:

Line 25: #include 
these don't seem used here. Can you move them to the .cc file where they are 
used?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/exec/kudu-scan-node.cc
File be/src/exec/kudu-scan-node.cc:

PS1, Line 274: thread_name
I think you already get the thread name in the 'name' parameter.


Line 275:   scanner_thread_counters_ =
I understand that this adds one counter per thread. Couldn't this be a lot of 
counters and thus lines we add to the profile? Have you considered adding a 
histogram of counters instead of one per thread?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/exec/kudu-scan-node.h
File be/src/exec/kudu-scan-node.h:

Line 24: #include 
these don't seem used here. Can you move them to the .cc file where they are 
used?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/runtime/plan-fragment-executor.cc
File be/src/runtime/plan-fragment-executor.cc:

Line 306:   plan_fragment_counters_ = ADD_THREAD_COUNTERS(profile(),
Does this have to be a member variable, i.e. does it have to be valid past the 
scope of this method?


http://gerrit.cloudera.org:8080/#/c/4561/1/be/src/runtime/plan-fragment-executor.h
File be/src/runtime/plan-fragment-executor.h:

Line 150:   ///Name of the plan fragment counter
Describe what the counter tracks. The comment and name don't seem to match.


Line 153:   ///Fragment thread counters
Can you explain what these count?


Line 154:   RuntimeProfile::ThreadCounters* plan_fragment_counters_;
Make this protected?


Line 156:   RuntimeProfile::ThreadCounters* plan_fragment_counters() const {
the variable seems to be used internally only, so I don't see why this getter 
is needed. However, I saw that pattern in other files, too, and if you want to 
follow it, maybe it's best to also have a setter to initialize the variable. 
Currently it's set by direct assignment and then referenced with the getter, 
which I think looks a bit confusing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b148eafb76319e727e8d0ef33c49b300b0c887f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

2016-09-29 Thread Lars Volker (Code Review)

Lars Volker has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
..

Patch Set 5:

(24 comments)

Thanks for all the help so far. I created a bunch of upstream Jiras and
replaced references to internal ones.

Please see my inline comments.

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

PS4, Line 24:
Can we rename this to DOWNLOAD_HADOOP_COMPONENTS instead? Or are they CDH
specific versions and that would make it worse?

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/build_thirdparty.sh
File bin/build_thirdparty.sh:

Line 46
This was unused, removed

http://gerrit.cloudera.org:8080/#/c/4187/1/bin/impala-config.sh
File bin/impala-config.sh:

PS1, Line 79:
Do we have plans to actually get rid of the CDH component dependencies? This
one seems to affect a lot more occurrences below. If anyone can shed some light
on it I'll go ahead and create a Jira for doing it.

http://gerrit.cloudera.org:8080/#/c/4187/3/bin/impala-config.sh
File bin/impala-config.sh:

Line 144
> There are so many different things we are doing with different "CDH"s, it m
Created IMPALA-4208 to track this.

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/impala-config.sh
File bin/impala-config.sh:

PS4, Line 77:
see comments in bootstrap_toolchain.py

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/save-version.sh
File bin/save-version.sh:

Line 24
This has been fixed separately in IMPALA-4116

http://gerrit.cloudera.org:8080/#/c/4187/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

PS4, Line 220:
:
IMPALA-4208

http://gerrit.cloudera.org:8080/#/c/4187/1/common/thrift/generate_metrics.py
File common/thrift/generate_metrics.py:

PS1, Line 49: parser.add_option("--output_mdl_version",
dest="output_mdl_version",
: metavar="IMPALA_VERSION",
default="2.8.0-SNAPSHOT",
: help="The Impala version that is written in the
output mdl.")
> Thanks. Let me know what they say and I'll create a Jira to track removal o
Did anything come out of this? For now I changed the version to the current
version in save-version.sh, so the 'cdh' is gone. :)

http://gerrit.cloudera.org:8080/#/c/4187/4/common/thrift/generate_metrics.py
File common/thrift/generate_metrics.py:

Line 50
I think we should have better scripts for version management, i.e. one script
that produces proper version files for all languages to include. For now I will
bump this to the current version in save-version.sh.

http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/api/pom.xml
File ext-data-source/api/pom.xml:

IMPALA-4225

http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/sample/pom.xml
File ext-data-source/sample/pom.xml:

IMPALA-4225

http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/test/pom.xml
File ext-data-source/test/pom.xml:

IMPALA-4225

http://gerrit.cloudera.org:8080/#/c/4187/4/fe/pom.xml
File fe/pom.xml:

IMPALA-4225

http://gerrit.cloudera.org:8080/#/c/4187/4/infra/deploy/deploy.py
File infra/deploy/deploy.py:

PS4, Line 23:
I doubt this is accurate, otherwise it could probably be removed, since this
quite an old version.

@mj, can you have a look? Should we move this script to some internal repo or
is it useful without CDH, too?

Let me know what to do and I'll open a Jira to track.

http://gerrit.cloudera.org:8080/#/c/4187/3/testdata/cluster/.gitignore
File testdata/cluster/.gitignore:

PS3, Line 1: kud
> I'm not sure about this one. I don't want to have to add one of these for e
Good point, removed it.

http://gerrit.cloudera.org:8080/#/c/4187/1/testdata/cluster/admin
File testdata/cluster/admin:

> Yes, I think I was wrong. There was a similar discussion in bin/start-impal
IMPALA-4208

http://gerrit.cloudera.org:8080/#/c/4187/4/testdata/cluster/admin
File testdata/cluster/admin:

IMPALA-4208

PS4, Line 28:
:
IMPALA-4208

http://gerrit.cloudera.org:8080/#/c/4187/4/testdata/datasets/functional/schema_constraints.csv
File testdata/datasets/functional/schema_constraints.csv:

PS4, Line 126:
Wouldn't this hold true for all versions of CDH (and all environments, really)?
Then again I don't understand why the comment is there in the first place. Can
decimal not be tested read-only?

http://gerrit.cloudera.org:8080/#/c/4187/4/tests/comparison/cluster.py
File tests/comparison/cluster.py:

IMPALA-4208

http://gerrit.cloudera.org:8080/#/c/4187/3/tests/comparison/leopard/impala_docker_env.py
File tests/comparison/leopard/impala_docker_env.py:

Line 34: DEFAULT_BRANCH_NAME = 'origin/cdh5-trunk'
> Whether IMPALA-4085 is fixed, we should remove these.
I don't know, I will ask on the mailing list.

By "remove these", do you mean the variables? Or the whole file?

http://gerrit.cloudera.org:8080/#/c/4187/4/tests/metadata/test_compute_stats.py

[Impala-ASF-CR] IMPALA-4206: Add column lineage regression test.

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4206: Add column lineage regression test.
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b164000c7b0ce7e2f296d168d75a6860f5963d8
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Internal Jenkins
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4206: Add column lineage regression test.

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4206: Add column lineage regression test.
..


IMPALA-4206: Add column lineage regression test.

The underlying issue was already fixed in IMPALA-3940.
This patch adds a new regression test to cover the IMPALA-4206.

Change-Id: I5b164000c7b0ce7e2f296d168d75a6860f5963d8
Reviewed-on: http://gerrit.cloudera.org:8080/4556
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
1 file changed, 44 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5b164000c7b0ce7e2f296d168d75a6860f5963d8
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Internal Jenkins

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Michael Ho has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4350/7/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

PS7, Line 171: put_list_.size
> Right, I understand that, but that doesn't mean Size() should return any ra
Actually, even with holding the write lock, there is still a window in which 
the get_list_size_ may be stale so the caller of Size() may see zero even if 
get_list_ is non-empty. The new patch fixes this race by setting get_list_size_ 
too before dropping write lock.


http://gerrit.cloudera.org:8080/#/c/4350/9/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

PS9, Line 201: 
> That's my preference but i don't feel too strongly.
Tried removing const but it affects callers of Size() which also has const. 
Don't wanna open another can of worm.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue
..

IMPALA-4026: Implement double-buffering for BlockingQueue

With recent changes to improve the parquet scanner's efficency,
row batches are produced more quickly, leading to higher contention
in the blocking queue shared between scanner threads and the scan
node. The contention happens between different producers (i.e.
the scanner threads) and also to a lesser extent, between the
scanner threads and the scan node.

This change addresses the contention between the scanner threads
and the scan node by splitting the queue into a 'get_list_' and
a 'put_list_'. The consumers will consume from 'get_list_' until
it's exhausted while the producers will enqueue into 'put_list_'
until it's full. When 'get_list_' is exhausted, the consumer will
atomically swap the 'get_list_' with 'put_list_'. This reduces
the contention: 'get_list_' and 'put_list_' are protected by two
different locks so callers of BlockingGet() only contends for the
'put_lock_' when 'put_list_' is empty.

With this change, primitive_filter_bigint_non_selective improves
by 33.9%, going from 1.60s to 1.06s

Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
---
M be/src/common/compiler-util.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/util/blocking-queue.h
A be/src/util/condition-variable.h
M be/src/util/thread-pool.h
6 files changed, 222 insertions(+), 66 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts

Michael Ho has posted comments on this change.

Change subject: IMPALA-4180: Synchronize accesses to 
RuntimeState::reader_contexts_
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

Line 304:   }
> reader_contexts_.clear()
Done


http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

Line 167:   void DeferIOContextUnregistration(DiskIoRequestContext* 
reader_context);
> AcquireReaderContext()?
Done


Line 170:   void UnregisterIOContexts();
> UnregisterReaderContexts()?
Done


http://gerrit.cloudera.org:8080/#/c/4558/1/testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test
File 
testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test:

Line 4: # whose children are scan nodes. When scan nodes are closed 
concurrently, make
> I don't think the last sentence is necessary. It's likely to become stale p
Rephrased the comment in the new patch.


http://gerrit.cloudera.org:8080/#/c/4558/1/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:

Line 131:   def test_single_node_topn(self, vector):
> This new test seems kind of misleading. The cause of this are two scan node
The new test is moved to single-node-nlj.test in the new patch. Rewrote the 
query to be NLJ which also exercises the same path.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts