[Impala-ASF-CR] IMPALA-5744: Add dummy 'use krpc' flag and create DataStream interface

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5744: Add dummy 'use_krpc' flag and create DataStream 
interface
..


Patch Set 7:

(10 comments)

Looks pretty close to me.

http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/data-stream-mgr-base.h
File be/src/runtime/data-stream-mgr-base.h:

PS7, Line 36: Thrfit and KRPC
: /// respectively. Remove this when we evenually get rid of the 
Thrift implementation and
: /// replace client references to this class with the KRPC's 
version of the
: /// DataStreamMgrBase.
same comments and typos as elsewhere.


http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/data-stream-mgr.h
File be/src/runtime/data-stream-mgr.h:

PS7, Line 22: #include "runtime/data-stream-mgr-base.h"
Move to line 33, with the other Impala includes.


http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/data-stream-recvr-base.h
File be/src/runtime/data-stream-recvr-base.h:

PS7, Line 33: Thrfit
typo


PS7, Line 34: respectively
remove (there's nothing in this sentence for them to be respective to).


PS7, Line 34: when we evenually get rid of the Thrift implementation and
: /// replace client references to this class with the KRPC's 
version of the
: /// DataStreamRecvrBase.
"in favor of the KRPC implementation when possible".


http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/data-stream-recvr.h
File be/src/runtime/data-stream-recvr.h:

PS7, Line 42: /// Single receiver of an m:n data stream.
This should say something about the fact that it's the thrift implementation.


http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

PS7, Line 460: ImpalaTestBackend
Is it hard to convert ImpalaTestBackend to accept a DataStreamMgrBase ?


http://gerrit.cloudera.org:8080/#/c/7542/7/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:

PS7, Line 93: enforced by
'part of' ?


PS7, Line 94: (This is due to the stream mgrs using different AddData() 
signatures)
Would remove this.


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

Line 52: }
return Status::OK()? Otherwise clang-tidy might complain. Here and elsewhere.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
..

IMPALA-5666: ASAN poisoning for MemPool and BufferPool

* Use ASAN_[UN]POISON_MEMORY_REGION to indicate to ASAN runtime that
  memory is not 'valid' from Impala's perspective (but still valid from
  kernel's perspective).

* Add this to MemPool and BufferPoolAllocator. For both, memory goes
  through the following lifecycle: when it is allocated from the OS and
  returned to the user, it must be unpoisoned. When the user returns it
  to a cache, it becomes poisoned. When the cache is freed to the OS, it
  must be unpoisoned again (otherwise future allocations elsewhere in
  the system might return poisoned memory).

* Also add checks to FreePool which uses a MemPool underneath, but has
  its own freelist cache. Fix a couple of bugs in free-pool-test that
  these checks uncovered.

Testing:

* Tests that only run if ASAN is enabled to confirm that poisoning
  catches simple use-after-return errors. These are 'death' tests which
  catch expected process exits.

Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
---
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/buffer-allocator.cc
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/free-list.h
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
10 files changed, 152 insertions(+), 17 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..


Patch Set 2:

(1 comment)

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

PS2, Line 172: ssl_cipher_list
> I'm wondering if we should default to "intermediate" compatibility:
It's a good idea, but I'm really keen not to break any existing clients. I'll 
leave a comment for 3.0!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#3).

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..

IMPALA-5696: Enable cipher configuration when using TLS / Thrift

The 'cipher suite' is a description of the set of algorithms used by SSL
and TLS to execute key exchange, encryption, message authentication, and
random number generation functions. SSL implementations allow the cipher
suite to be configured so that ciphers may be removed from the whitelist
if they are shown to be weak.

* Add a flag --ssl_cipher_list which controls cipher selection for both
  thrift servers and clients. Default is blank, which means use all
  available cipher suites.
* Add ThriftServerBuilder to simplify construction of
  ThriftServers (whose constructors were otherwise getting very long).

Testing: new tests added to thrift-server-test. Test cases added follow:

* A client cannot connect to a server which does not have any ciphers in
  common with it.
* If ciphers are identical on clients and servers, that ssl connections
  can be made.
* Bad cipher strings lead to errors on both client and server.

Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
---
M be/src/benchmarks/network-perf-benchmark.cc
M be/src/catalog/catalogd-main.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/data-stream-test.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
A be/src/testutil/scoped-flag-setter.h
M be/src/util/webserver-test.cc
13 files changed, 427 insertions(+), 142 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5666: ASAN poisoning for MemPool and BufferPool

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5666: ASAN poisoning for MemPool and BufferPool
..


Patch Set 2:

(3 comments)

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

Line 24:   these checks uncovered.
> Did you run the full test suite with ASAN?
Not yet, will do (and update) before I commit. Have run a good subset of the 
be-tests and some full queries.


http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-allocator.cc
File be/src/runtime/bufferpool/buffer-allocator.cc:

Line 400: // Ensure that the memory is unpoisoned when it's next allocated 
by the system.
> Is this one needed? Won't the system allocator call free(), which will pois
You would have thought so, but I thought I saw one case where memory returned 
to the system was re-allocated as poisoned. Better safe than sorry.


http://gerrit.cloudera.org:8080/#/c/7591/1/be/src/runtime/bufferpool/buffer-pool.h
File be/src/runtime/bufferpool/buffer-pool.h:

Line 414:   void Poison() { ASAN_POISON_MEMORY_REGION(data(), len()); }
> I doubt it makes much of a difference but it would be nice if we could make
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8d5a28dfee2b7c631981aac75524bde9acc0b36a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

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

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

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


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

A few miscellaneous changes to allow kudu_util to compile with Impala.

Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.

Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.

Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.

Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.

Finally, a few changes to allow compilation on RHEL5:

* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
  it does not compile due to other #includes in env_posix.cc. We disable
  the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
  it doesn't exist.

In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.

This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.

Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/common/config.h.in
A be/src/common/kudu_version.cc
M be/src/common/logging.cc
M be/src/exec/kudu-util.h
A be/src/kudu/gutil
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/cache.cc
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/locks.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M be/src/kudu/util/net/socket.cc
M be/src/kudu/util/subprocess.cc
M bin/run_clang_tidy.sh
21 files changed, 203 insertions(+), 46 deletions(-)

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



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

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

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

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

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


Patch Set 27: Code-Review+2

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

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

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1:

Does the toolchain version need to be bumped as well?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

PS1, Line 335: DCHECK(ValidateCurrentIndex());
Just a passing comment - if this DCHECK fails, all it will tell us via the logs 
is that ValidateCurrentIndex() was false, because the DCHECK will just print 
the condition that failed. 

This is useful for debugging, but not as useful as knowing *how* it failed (off 
by one error? was the idx badly initialized?). So my suggestion is to have 
ValidateCurrentIndex() look more like this:

   void ValidateCurrentIndex() {
 DCHECK_LE(0, current_idx_);
 DCHECK_GT(streams_.size(), current_idx_);
   }

The compiler will figure out in release mode that the method call does nothing, 
and should be able to optimize it out.


http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.h
File be/src/exec/scanner-context.h:

PS1, Line 332: return current_idx_
consider case where current_idx_ < 0 as well?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1: Code-Review+2

No, if it's already in the current toolchain version you don't need a separate 
build.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5781: Only use TLSv1.0-compatible ciphers for tests

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5781: Only use TLSv1.0-compatible ciphers for tests
..

IMPALA-5781: Only use TLSv1.0-compatible ciphers for tests

If we ask OpenSSL to use a cipher suite that's not compatible with
TLSv1.0, it will fail on machines where TLSv1.1+ is not
supported (i.e. those with OpenSSL v1.0.0).

Fix tests to only use TLSv1.0-compatible cipher suites, picked from
https://wiki.openssl.org/index.php/Manual:Ciphers(1)#TLS_v1.0_cipher_suites.

Confirmed that tests start servers with TLSv1.0 support. Before this
patch, servers would be silently upgraded to TLSv1.2 only (i.e. the
minimum version that supported the requested cipher suite).

Change-Id: Id66508040bcc7745b7c68b62ace71ae1d394c1b4
---
M be/src/rpc/thrift-server-test.cc
1 file changed, 15 insertions(+), 7 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5744: Add dummy 'use krpc' flag and create DataStream interface

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5744: Add dummy 'use_krpc' flag and create DataStream 
interface
..


Patch Set 1:

(1 comment)

Quick question before I get stuck in - did you evaluate the complexity of 
building this class hierarchy vs having the users of the datastream interfaces 
keeping a pointer to both possible implementations, and using FLAGS_use_krpc to 
decide which one to call into?

It would be a bit tedious in, e.g. exchange-node.cc - but it would be 
self-contained, and it would be easy to undo the changes when we standardize on 
one solution. With the class hierarchy approach, I'm a bit concerned about the 
amount of code we introduce that will ultimately be removed.

http://gerrit.cloudera.org:8080/#/c/7542/1/be/src/runtime/sender-queue-base.h
File be/src/runtime/sender-queue-base.h:

PS1, Line 28: #include "common/names.h"
this shouldn't be included in a .h file


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

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

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

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


Patch Set 2:

Any takers for a review? This patch reduces the be-test end-to-end time from 
13.5 minutes to 9.5 minutes on my machine (and there's much more parallelism to 
be had if we break up some of the larger test cases).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I865db25b07728f3886133316ded5122c60490967
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

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

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#2).

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

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

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

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

ctest -R "foo_sharded"

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

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

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

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


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

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

[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#12).

Change subject: IMPALA-4669: [SECURITY] Add security library to build
..

IMPALA-4669: [SECURITY] Add security library to build

* Minor compilation fix
* Set toolchain version to include gflags 2.2.0 and glog 0.3.4-p2
* Look for krb5 headers in toolchain (but dynamically link against
  libraries)
* Link against openssl from the toolchain if 1.0.1 or higher not found
  on build machine.

Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/kudu/security/init.cc
M be/src/kudu/security/token_verifier.cc
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M cmake_modules/FindKerberos.cmake
7 files changed, 44 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4669: [SECURITY] Add security library to build
..


Patch Set 12:

This patch builds on RHEL5.5 and 6.0, debian 7 and 8, ubuntu 12.04 and 14.04, 
and SLES11 and 12.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5715: (potential mitigation) don't destroy MemTracker during Close()

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5715: (potential mitigation) don't destroy MemTracker 
during Close()
..


Patch Set 1:

Thanks!

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

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

[Impala-ASF-CR] IMPALA-5743: Support TLS version configuration for Thrift servers

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5743: Support TLS version configuration for Thrift 
servers
..

IMPALA-5743: Support TLS version configuration for Thrift servers

* Add --ssl_minimum_version which controls the minimum SSL/TLS version
  that clients and servers will use when negotiating a secure
  connection.
* Two kinds of version specification are allowed: 'TLSv1.1' enables
  TLSv1.1 and all subsequent verisons. 'TLSv1.1_only' enables only
  TLSv1.1. The latter is not exposed in user-facing text as it is
  typically only used for testing.
* Handle case where platform may not support TLSv1.1 or v1.2 by checking
  OpenSSL version number.
* Bump Thrift toolchain version to -p10.

Testing:
* New tests in thrift-server-test.cc. In particular, test all 36
  configurations of client and server protocol versions, and ensure that
  the expected successes or failures are seen.

Change-Id: I4c68a6c9658ddbfbe8025f2021fd5ed7a9dec5a5
---
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-client.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-server.cc
M bin/impala-config.sh
7 files changed, 176 insertions(+), 28 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5774: Prevent FindInSet() from reading off end of string

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5774: Prevent FindInSet() from reading off end of string
..

IMPALA-5774: Prevent FindInSet() from reading off end of string

Change-Id: I541c8e6bb712e380f9610d6bfa35e2d515a31d1d
---
M be/src/exprs/string-functions-ir.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

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

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

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

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


Patch Set 2:

Only in the sense that I was hoping for some feedback on the approach (e.g. is 
it a good idea, running N test processes at the same time, logging to different 
places - will it be more painful to find an error message after the fact)?

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

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

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..


Patch Set 6: Code-Review+2

Forgot to add the couple of lines of code to enable this in the BE and FE 
servers :/ Done now, carry +2.

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

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

[Impala-ASF-CR] IMPALA-5744: Add 'use krpc' flag and create DataStream interface

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5744: Add 'use_krpc' flag and create DataStream interface
..


Patch Set 9: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d52245154e910529a68f53049520238eca16241
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Matthew Jacobs, Sailesh Mukil,

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

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

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

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..

IMPALA-5696: Enable cipher configuration when using TLS / Thrift

The 'cipher suite' is a description of the set of algorithms used by SSL
and TLS to execute key exchange, encryption, message authentication, and
random number generation functions. SSL implementations allow the cipher
suite to be configured so that ciphers may be removed from the whitelist
if they are shown to be weak.

* Add a flag --ssl_cipher_list which controls cipher selection for both
  thrift servers and clients. Default is blank, which means use all
  available cipher suites.
* Add ThriftServerBuilder to simplify construction of
  ThriftServers (whose constructors were otherwise getting very long).

Testing: new tests added to thrift-server-test. Test cases added follow:

* A client cannot connect to a server which does not have any ciphers in
  common with it.
* If ciphers are identical on clients and servers, that ssl connections
  can be made.
* Bad cipher strings lead to errors on both client and server.

Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
---
M be/src/benchmarks/network-perf-benchmark.cc
M be/src/catalog/catalogd-main.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/data-stream-test.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
A be/src/testutil/scoped-flag-setter.h
M be/src/util/webserver-test.cc
13 files changed, 440 insertions(+), 143 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5394: Handle blocked HS2 connections

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5394: Handle blocked HS2 connections
..


Patch Set 3:

My instinct would be just to use the size of tasks_ to control the coordination.

In SetupConnection(), line 168 or so:

  { 
Synchronized s(tasksMonitor_);
if (tasks_.size() > max) tasksMonitor_.wait();
tasks_.insert(task);
  }

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Sherman 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1891: Statestore won't send deletions in initial non-delta topic

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-1891: Statestore won't send deletions in initial 
non-delta topic
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7527/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS1, Line 531: UNLIKELY
this isn't perf critical, so let's remove UNLIKELY for now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fc525e1f3d960d642fc6356abb75f744cab7c33
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..

IMPALA-5696: Enable cipher configuration when using TLS / Thrift

The 'cipher suite' is a description of the set of algorithms used by SSL
and TLS to execute key exchange, encryption, message authentication, and
random number generation functions. SSL implementations allows the
cipher suite to be configured so that ciphers may be removed from the
whitelist if they are shown to be weak.

* Add a flag --ssl_cipher_list which controls cipher selection for both
  thrift servers and clients. Default is blank, which means use all
  available cipher suites.
* Add ThriftServerBuilder to simplify construction of
  ThriftServers (whose constructors were otherwise getting very long).

Testing: new tests added to thrift-server-test. Test cases added follow:

* A client cannot connect to a server which does not have any ciphers in
  common with it.
* If ciphers are identical on clients and servers, that ssl connections
  can be made.
* Bad cipher strings lead to errors on both client and server.

Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
---
M be/src/benchmarks/network-perf-benchmark.cc
M be/src/catalog/catalogd-main.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/data-stream-test.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
A be/src/testutil/scoped-flag-setter.h
M be/src/util/webserver-test.cc
13 files changed, 399 insertions(+), 144 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Matthew Jacobs, Sailesh Mukil,

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

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

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

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..

IMPALA-5696: Enable cipher configuration when using TLS / Thrift

The 'cipher suite' is a description of the set of algorithms used by SSL
and TLS to execute key exchange, encryption, message authentication, and
random number generation functions. SSL implementations allow the cipher
suite to be configured so that ciphers may be removed from the whitelist
if they are shown to be weak.

* Add a flag --ssl_cipher_list which controls cipher selection for both
  thrift servers and clients. Default is blank, which means use all
  available cipher suites.
* Add ThriftServerBuilder to simplify construction of
  ThriftServers (whose constructors were otherwise getting very long).

Testing: new tests added to thrift-server-test. Test cases added follow:

* A client cannot connect to a server which does not have any ciphers in
  common with it.
* If ciphers are identical on clients and servers, that ssl connections
  can be made.
* Bad cipher strings lead to errors on both client and server.

Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
---
M be/src/benchmarks/network-perf-benchmark.cc
M be/src/catalog/catalogd-main.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/data-stream-test.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
A be/src/testutil/scoped-flag-setter.h
M be/src/util/webserver-test.cc
13 files changed, 430 insertions(+), 142 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5484: Fix LICENSE issues discovered by IPMC in 2.9 vote

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5484: Fix LICENSE issues discovered by IPMC in 2.9 vote
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f98d1b2f514d7afdee8d86a45167905b272ca4d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..


Patch Set 5: Code-Review+2

Fix some clang-tidy warnings in the tests.

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

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

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 2: Code-Review+2

(3 comments)

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

Line 7: IMPALA-5108:idle_session_timeout kicks in later than expected
nit: put a space after ':'


PS2, Line 11: ~1 min
that's only a default - would be clearer to the reader to say "Sessions could 
get closed some time after their timeout expires, because the session 
expiration thread only woke up every session-timeout/2 seconds."


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

PS2, Line 1817: // Sleep for a second before checking whether an active session
  : // can be expired or not.
nit: can you wrap this to 90 chars?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

(4 comments)

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

Line 556:   value->AddMember("num_instances", fragments_.size(), 
document->GetAllocator());
> I think we should include some form of total_ranges_complete_ in this page.
I couldn't find a very easy way to plumb it through. Perhaps we could leave 
that to a follow-on patch?


PS1, Line 569: status_
> use GetStatus()? It takes the BackendState::lock_
Done


http://gerrit.cloudera.org:8080/#/c/7711/1/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

PS1, Line 710: request_state
> UNLIKELY(request_state.get()...) ?
I think UNLIKELY() is best used when we have branches whose mispredictions are 
expensive. That's not really the case here.


http://gerrit.cloudera.org:8080/#/c/7711/1/www/query_backends.tmpl
File www/query_backends.tmpl:

Line 57: 
> Does it make sense to add a checkbox to show only backends with unfinished 
The table can be sorted so that unfinished backends can be sorted to the top.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#3).

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..

IMPALA-5811: Add 'backends' tab to query details pages

Add a 'backends' tab to query details pages which shows:

  * host
  * total number of fragment instances for that query on that backend
  * number of still-running fragment instances
  * if the backend is complete (i.e. all instances finished)
  * peak memory consumption
  * the time, in ms, since a status report was received at the
  * coordinator from that backend.

The table refreshes itself every second, controllable by a check-box. If
the query has completed, no information is displayed.

Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
A www/query_backends.tmpl
M www/query_detail_tabs.tmpl
8 files changed, 172 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has abandoned this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..

IMPALA-5811: Add 'backends' tab to query details pages

Add a 'backends' tab to query details pages which shows:

  * host
  * total number of fragment instances for that query on that backend
  * number of still-running fragment instances
  * if the backend is complete (i.e. all instances finished)
  * peak memory consumption
  * the time, in ms, since a status report was received at the
  * coordinator from that backend.

The table refreshes itself every second, controllable by a check-box. If
the query has completed, no information is displayed.

Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
A www/query_backends.tmpl
M www/query_detail_tabs.tmpl
8 files changed, 171 insertions(+), 7 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 1:

Presumably the query was a DDL statement, so there was no coordinator. Thanks 
for the report, hopefully fixed now. 

Screenshot at 
https://gist.githubusercontent.com/henryr/1a50fc1ec7474353b84343815c9f867f/raw/f23d873c8d39c10dc00ffc4a1c30a9dbec3e0251/screen-shot.png

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..

IMPALA-5811: Add 'backends' tab to query details pages

Add a 'backends' tab to query details pages which shows:

  * host
  * total number of fragment instances for that query on that backend
  * number of still-running fragment instances
  * if the backend is complete (i.e. all instances finished)
  * peak memory consumption
  * the time, in ms, since a status report was received at the
  * coordinator from that backend.

The table refreshes itself every second, controllable by a check-box. If
the query has completed, no information is displayed.

Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
A www/query_backends.tmpl
M www/query_detail_tabs.tmpl
8 files changed, 171 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 

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

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

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


Patch Set 2:

Thanks for the review, btw - I'm working on some ergonomic improvements before 
resubmitting which is why this is taking a little while to resurface.

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

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

[Impala-ASF-CR] IMPALA-4786: Clean up how ImpalaServers are created

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-4786: Clean up how ImpalaServers are created
..

IMPALA-4786: Clean up how ImpalaServers are created

ImpalaServer had to be created via an awkward CreateImpalaServer()
method that took a lot of arguments. This patch refactors that code (in
anticipation of some KRPC changes) to follow a more normal lifecycle:

1. ImpalaServer* server = new ImpalaServer(ExecEnv*)
2. RETURN_IF_ERROR(server->Init()) // for error-returning init operations
3. RETURN_IF_ERROR(server->Start())
4. server->Join()

Also add ExecEnv::Init(), and move calls to ExecEnv::StartServices() to
ImpalaServer::StartServices(). This captures a dependency that KRPC will
rely on - where initialization of both ExecEnv and ImpalaServer need to
happen before services are started.

This sets up a clean-up of InProcessImpalaServer, which is too heavy for
the work that it does. That work is deferred to a follow-on patch.

Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae
---
M be/src/exprs/expr-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/util/hdfs-util-test.cc
10 files changed, 108 insertions(+), 137 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d55fbe0f4f7a1fd48993da46863b66e521feaae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(5 comments)

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

PS1, Line 1817:   
double space


PS1, Line 1817: that
whether


PS1, Line 1819: int64_t session_timeout_min_ms = 1000;
Rename this to something like sleep_time_ms , and make it a constant - e.g.:

  const int64_t SLEEP_TIME_MS = 1000;


PS1, Line 1844: FLAGS_use_local_tz_for_unix_timestamp_conversions = true;
This is very dangerous: what if some other thread reads 
FLAGS_use_local_tz_for_unix_timestamp_conversions concurrently?

I'd leave this change out for now.


http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 45: num_clients
constants should be ALL_CAPS


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5811: Add 'backends' tab to query details pages

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5811: Add 'backends' tab to query details pages
..


Patch Set 2:

That's deliberate - preserve the last seen set of states. I could change 
something to add an indicator if the coordinator has stopped responding, but 
that would be pretty tricky to do. Note that this behaviour is the same as 
other pages that simply stop updating when the query is done.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5b3b0fb8f4188da56da593199f41ce6fab99767
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

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

PS1, Line 20: Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
please remove this line and resubmit - Gerrit uses this as its unique ID for 
patch sets, so it thinks that there's two different patches for this one issue.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5800: Configure Squeasel's cipher suite and TLS version

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5800: Configure Squeasel's cipher suite and TLS version
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7679/2/be/src/thirdparty/squeasel/squeasel.c
File be/src/thirdparty/squeasel/squeasel.c:

PS2, Line 4275: ctx->config[SSL_CIPHERS]
> In our case, this will never be NULL right? Since our ssl_cipher_list flag 
See webserver.cc line 269 in this patch - we don't set the option if the cipher 
flag is empty.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23fb17257098992c65e50c1e83a905c9a85db6a2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5800: Configure Squeasel's cipher suite and TLS version

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5800: Configure Squeasel's cipher suite and TLS version
..


Patch Set 1:

(2 comments)

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

PS1, Line 10: https://github.com/cloudera/squeasel/commit/1e5f611
> I would add a reference to this as well, to avoid confusion:
Not to be pedantic, but this conveys what I'm trying to say: I took squeasel as 
of this particular commit, i.e. the snapshot of the file at this commit as the 
sum of all the commits before it. I've tried a slight rewording.


http://gerrit.cloudera.org:8080/#/c/7679/2/be/src/util/webserver-test.cc
File be/src/util/webserver-test.cc:

PS2, Line 251: SslBadCipherSuite
> Thanks for adding the test. This shouldn't be called "SslBadCipherSuite" no
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23fb17257098992c65e50c1e83a905c9a85db6a2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5800: Configure Squeasel's cipher suite and TLS version

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#3).

Change subject: IMPALA-5800: Configure Squeasel's cipher suite and TLS version
..

IMPALA-5800: Configure Squeasel's cipher suite and TLS version

* Import Squeasel as of
  https://github.com/cloudera/squeasel/commit/1e5f611, which adds TLS
  version and cipher suite configuration.

* Plumb through --ssl_minimum_version and --ssl_cipher_list to the
  Squeasel options in webserver.cc.

Testing: manually confirm that webserver connections have desired
protocol version and cipher suite. Add two tests to webserver-test to
check that misconfiguration leads to a failure.

Change-Id: I23fb17257098992c65e50c1e83a905c9a85db6a2
---
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/webserver-test.cc
M be/src/util/webserver.cc
3 files changed, 80 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23fb17257098992c65e50c1e83a905c9a85db6a2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5158,IMPALA-5236: account for unused buffer pool reservations

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5158,IMPALA-5236: account for unused buffer pool 
reservations
..


Patch Set 6:

FWIW (and this is veering away from an Apache discussion), I thought end-user 
tools like Cloudera Manager were supposed to do the curation of what was 
actionable vs not. That is, they provide the smarts to do things like health 
checking and so on. Impala's job should just be to expose its metrics and their 
metadata - not to be its own ops tool.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idb1fa3110dc893321f9f4e8ced6b7ede12194dad
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#12).

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..

IMPALA-4669: [KRPC] Add kudu_rpc library to build

Import FindKRPC.cmake from Apache Kudu.

One minor linking change to krpc/CMakeLists.txt.

Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
---
M CMakeLists.txt
M be/CMakeLists.txt
A cmake_modules/FindKRPC.cmake
3 files changed, 129 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..


Patch Set 12:

This patch builds on all platforms we test on here at Cloudera.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5775: (Addendum) Make SSL cluster actually come up in test client ssl.py

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5775: (Addendum) Make SSL cluster actually come up in 
test_client_ssl.py
..

IMPALA-5775: (Addendum) Make SSL cluster actually come up in test_client_ssl.py

The non-wildcard certs in test_client_ssl.py require that the hostname
of the process is 'localhost' for clients to validate them. This wasn't
the case for one test, and so the cluster wouldn't actually
start. Although the test would still pass (because the shell wasn't
actually checking the certificate), it's better hygiene to have the
cluster correctly configured to make sure we're testing what we think we
are.

Testing: test continues to pass

Change-Id: Idad8bbf3b8be853d3406bcbaed24909501500ea9
---
M tests/custom_cluster/test_client_ssl.py
1 file changed, 8 insertions(+), 3 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Michael Ho,

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

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

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

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..

IMPALA-4669: [KRPC] Add kudu_rpc library to build

Import FindKRPC.cmake from Apache Kudu.

Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/kudu/rpc/CMakeLists.txt
A cmake_modules/FindKRPC.cmake
4 files changed, 133 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5800: [Addendum] Fix bad import of squeasel.c

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5800: [Addendum] Fix bad import of squeasel.c
..

IMPALA-5800: [Addendum] Fix bad import of squeasel.c

The previous commit for IMPALA-5800 imported a slightly old version of
squeasel.c from the upstream project. In particular, the strings used to
choose the TLS version were incompatible with Impala's. This patch
corrects the error.

Change-Id: I03b76f2d3b3e2e6133e5c57a4f33ac315c889e15
---
M be/src/thirdparty/squeasel/squeasel.c
1 file changed, 9 insertions(+), 10 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5800: [Addendum] Fix bad import of squeasel.c

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-5800: [Addendum] Fix bad import of squeasel.c
..

IMPALA-5800: [Addendum] Fix bad import of squeasel.c

The previous commit for IMPALA-5800 imported a slightly old version of
squeasel.c from the upstream project. In particular, the strings used to
choose the TLS version were incompatible with Impala's. This patch
corrects the error.

Testing: Add tests to make sure that the expected flags are supported by
Squeasel.

Change-Id: I03b76f2d3b3e2e6133e5c57a4f33ac315c889e15
---
M be/src/thirdparty/squeasel/squeasel.c
M be/src/util/webserver-test.cc
2 files changed, 33 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I03b76f2d3b3e2e6133e5c57a4f33ac315c889e15
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] Propagate HAVE PIPE2 compile time value to files that use it

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: Propagate HAVE_PIPE2 compile time value to files that use it
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cdc343da35a34be8d95fbea3543d080dbc1ec29
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1972/IMPALA-3882: Fix QueryExecState::lock contention

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-1972/IMPALA-3882: Fix QueryExecState::lock_ contention
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6707/3/www/query_plan.tmpl
File www/query_plan.tmpl:

PS3, Line 42: {{?plan_metadata_unavailable}}
:  Query plan information is not yet available since the 
planning hasn't finished.
: {{/plan_metadata_unavailable}}
: 
: {{^plan_metadata_unavailable}}
Since the webpage refreshes every few seconds, it would be best to have this 
say something like "Plan not yet available. Page will update when query 
planning completes".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie44daa93e3ae4d04d091261f3ec4891caffe8026
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

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

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

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


Patch Set 10:

Thanks for the review, Tim - do you have any further comments?

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

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

[Impala-ASF-CR] IMPALA-4890/5143: Coordinator race involving TearDown()

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4890/5143: Coordinator race involving TearDown()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6897/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 895
This TODO still seems relevant - WaitForBackendCompletion() shouldn't happen on 
GetNext(), because that could block the client from seeing final results for a 
long time.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I457a6424a0255c137336c4bc01a6e7ed830d18c7
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5174: Bump gflags to 2.2.0-p1

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5174: Bump gflags to 2.2.0-p1
..

IMPALA-5174: Bump gflags to 2.2.0-p1

This gflags patch adds DEFINE_int32_hidden() etc. macros, which suppress
flags from appearing in /varz, --help and other flag enumerations.

Our toolchain glog is statically linked against gflags, and therefore
had to be rebuilt, however its version number did not change. You will
likely need to do the following:

rm -rf ${IMPALA_TOOLCHAIN_DIR}/glog-0.3.4-p2/

before running bin/bootstrap_toolchain.py, otherwise building Impala may
fail with a linking error.

Change-Id: Ibc09a750879a8eae8b3549b9438241cb7c4448ed
---
M bin/impala-config.sh
1 file changed, 2 insertions(+), 2 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5331: Use new libHDFS API to address "Unknown Error 255"

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5331: Use new libHDFS API to address "Unknown Error 255"
..


Patch Set 1:

If hdfsGetLastExceptionRootCause() always contains relevant information, I 
think we should include it in all cases. What's to be gained by leaving it out?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I181e316ed63b70b94d4f7a7557d398a931bb171d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)
..


Patch Set 2: Verified+1

Passed a full toolchain build.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48d718bac3dbf548cdaefc70f8f497bbebe30da6
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)
..


IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)

See https://github.com/HenryR/gflags/tree/hidden_flags

for the commit that this patch was generated from.

Change-Id: I48d718bac3dbf548cdaefc70f8f497bbebe30da6
---
M buildall.sh
A 
source/gflags/gflags-2.2.0-patches/0001-Allow-hidden-flags-e.g.-DEFINE_int32_hidden.patch
2 files changed, 346 insertions(+), 2 deletions(-)

Approvals:
  Henry Robinson: Verified
  Matthew Jacobs: Looks good to me, but someone else must approve
  Dan Hecht: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I48d718bac3dbf548cdaefc70f8f497bbebe30da6
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-5480: Improve missing filters message

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5480: Improve missing filters message
..

IMPALA-5480: Improve missing filters message

Replace:

"Only following filters arrived: , waited 20ms"

with:

"Not all filters arrived (arrived: [], missing: [0]), waited 20ms"

This shows up in the logs and the profile.

Change-Id: Ia87fabdfb591f33343020c4f3bb17dc0fb011046
Testing: ran manually and read the message.
---
M be/src/exec/hdfs-scan-node-base.cc
1 file changed, 8 insertions(+), 3 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4856: Port ImpalaInternalService to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has abandoned this change.

Change subject: IMPALA-4856: Port ImpalaInternalService to KRPC
..


Abandoned

Superseded by https://gerrit.cloudera.org/#/c/7103/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I95229290566a8ccffd80ed2d74c1c57cf1479238
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Anonymous Coward #168
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-4856: Port data stream service to KRPC
..

IMPALA-4856: Port data stream service to KRPC

This patch ports the data-flow parts of ImpalaInternalService to KRPC.

* ImpalaInternalService is split into two services. The first,
  ImpalaInternalService, deals with control messages for plan fragment
  instance execution, cancellation and reporting, and remains
  implemented in Thrift for now. The second, DataStreamService, handles
  large-payload RPCs for transmitting runtime filters and row batches
  between hosts.

* In the DataStreamService, all RPCs use 'native' protobuf. The
  DataStreamService starts on the port previously reserved for the
  StatestoreSubscriberService (which is also a KRPC service), to avoid
  having to configure another port when starting Impala. When the
  ImpalaInternalService is ported to KRPC, all services will run on one
  port.

* To support needing to address two different backend services, a data
  service port has been added to TBackendDescriptor.

* This patch adds support for asynchronous RPCs to the RpcMgr and Rpc
  classes. Previously, Impala used fixed size thread pools + synchronous
  RPCs to achieve some parallelism for 'broadcast' RPCs like filter
  propagation, or a dedicated per-sender+receiver pair thread on the
  sender side in the DataStreamSender case. In this patch, the
  PublishFilter() and TransmitData() RPCs are sent asynchronously using
  KRPC's thread pools.

* The TransmitData() protocol has changed to adapt to asynchronous
  RPCs. The full details are in data-stream-mgr.h.

* As a result, DataStreamSender no longer creates a
  thread-per-connection on the sender side.

* Both tuple transmission and runtime filter publication use sidecars to
  minimise the number of copies and serialization steps required.

* Also include a fix for KUDU-2011 that properly allows sidecars to be
  shared between KRPC and the RPC caller (fixing IMPALA-5093, a
  corruption bug).

* A large portion of this patch is the replacement of TRowBatch with its
  Protobuf equivalent, RowBatchPb. The replacement is a literal port of
  the data structure, and row-batch-test, row-batch-list-test and
  row-batch-serialize-benchmark continue to execute without major logic
  changes.

* Simplify FindRecvr() logic in DataStreamManager. No-longer need to
  handle blocking sender-side, so no need for complex promise-based
  machinery. Instead, all senders with no receiver are added to a
  per-receiver list, which is processed when the receiver arrives. If it
  does not arrive promptly, the DataStreamManager cleans them up after
  FLAGS_datastream_sender_timeout_ms.

* This patch also begins a clean-up of how ImpalaServer instances are
  created (by removing CreateImpalaServer), and clarifying the
  relationship between ExecEnv and ImpalaServer. ImpalaServer now
  follows the standard construct->Init()->Start()->Join() lifecycle that
  we use for other services.

* Ensure that all addresses used for KRPCs are fully resolved, avoiding
  the need to resolve them for each RPC.

TESTING
---

* New tests added to rpc-mgr-test.

TO DO
-

* Re-enable throughput and latency measurements per data-stream sender
  when that information is exposed from KRPC (KUDU-1738).

* TLS and Kerberos are still not supported by KRPC in this patch.PART1 DSS

Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b
---
M .clang-format
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/benchmarks/row-batch-serialize-benchmark.cc
M be/src/common/init.cc
M be/src/common/status.cc
M be/src/common/status.h
M be/src/exprs/expr-test.cc
M be/src/kudu/rpc/rpc_sidecar.cc
M be/src/kudu/rpc/rpc_sidecar.h
M be/src/rpc/CMakeLists.txt
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/common.proto
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/rpc/rpc.h
D be/src/runtime/backend-client.h
M be/src/runtime/client-cache-types.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/data-stream-sender.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch-serialize-test.cc
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-state.cc
M 

[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4856: Port data stream service to KRPC
..


Patch Set 1:

(10 comments)

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

Line 61:   bool unused = false;
> Doesn't the contract for FindRecvr() state that we need to hold 'lock_' bef
Done


Line 105:   EarlySendersList waiters;
> Add brief comment:
Done


PS1, Line 123: for (int32_t sender_id: waiters.closing_senders) 
recvr->RemoveSender(sender_id);
> According to the header comment in data-stream-mgr.h, a sender shouldn't be
Done


PS1, Line 300: early_senders_
> Assume the following case:
The sender fragment instance would fail, and then the coordinator should cancel 
the receiver. 

I believe there's an outstanding issue where, if the coordinator fails to 
cancel a fragment instance, the fragment instance will not fail itself. I'm 
going to file a JIRA for that, but it's unrelated to KRPC.


Line 321: // Wait for 10s
> Add a brief comment stating that this is to check if the DataStreamMgr is b
Done


http://gerrit.cloudera.org:8080/#/c/7103/1/be/src/runtime/data-stream-mgr.h
File be/src/runtime/data-stream-mgr.h:

PS1, Line 81: will do one of three things
> nit: would be nice to format them as bullet points.
Done


PS1, Line 83: if the buffer is full
> "if the batch queues are full"?
Done


PS1, Line 87: the sender
> "the sender along with its payload" ?
Done


Line 224:   /// has not yet prepared 'payload' is queued until it arrives, or 
is timed out. If the
> nit: been prepared,
Done


http://gerrit.cloudera.org:8080/#/c/7103/1/be/src/service/impala-server.h
File be/src/service/impala-server.h:

PS1, Line 255: void UpdateFilter
> Leave a TODO stating that this should move to query-state.h/cc after IMPALA
I'm going to leave that for now, since I don't want to make design decisions 
for IMPALA-3825 in this patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5537: Retry RPC on somes exceptions with SSL connection

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5537: Retry RPC on somes exceptions with SSL connection
..


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7229/5/be/src/testutil/fault-injection-util.cc
File be/src/testutil/fault-injection-util.cc:

PS5, Line 78: 
Was this string wrong before, or did something change? If it was wrong, were 
the tests silently passing?


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

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

[Impala-ASF-CR] Bump thrift toolchain version to 0.9.0-p9

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: Bump thrift toolchain version to 0.9.0-p9
..

Bump thrift toolchain version to 0.9.0-p9

The toolchain has already been built at least once with 0.9.0-p9
installed, so a good idea to bump this now in case we forget next time
someone wants to upgrade a dependency.

Change-Id: I3ba43d0ca24445c69b2931aff0b7fc92ebd5edbf
---
M bin/impala-config.sh
1 file changed, 2 insertions(+), 2 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-1474: Add a metric for running queries

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-1474: Add a metric for running queries
..

IMPALA-1474: Add a metric for running queries

impala-server.num-queries-in-flight is a gauge that counts all running
queries (including DDL and DML statements). Queries waiting to be
unregistered are included in the count.

Testing: checked completed, in-flight and cancelled queries. Added some
coverage to the HS2 test suite, and the query lifecycle tests.

Change-Id: I41bc88d0e91427ca2fd70136dc6e06fa7a2663da
---
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/metrics.json
M tests/hs2/hs2_test_suite.py
M tests/hs2/test_fetch.py
M tests/query_test/test_lifecycle.py
13 files changed, 119 insertions(+), 77 deletions(-)


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

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

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7224/1/source/krb5/build.sh
File source/krb5/build.sh:

PS1, Line 2: 2015
> nit: 2017
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..


IMPALA-5526: Add krb5 1.15.1 to toolchain

Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
---
M buildall.sh
A source/krb5/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)

Approvals:
  Henry Robinson: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..

IMPALA-5526: Add krb5 1.15.1 to toolchain

Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
---
M buildall.sh
A source/krb5/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/24/7224/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..


Patch Set 2:

Carry +2. This already passed a full toolchain build.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-5526: Add krb5 1.15.1 to toolchain

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5526: Add krb5 1.15.1 to toolchain
..

IMPALA-5526: Add krb5 1.15.1 to toolchain

Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
---
M buildall.sh
A source/krb5/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5237f8cba41f91e70f0a915f841db51b5cb93655
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 

[Impala-ASF-CR] IMPALA-5537: Retry RPC on somes exceptions with SSL connection

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5537: Retry RPC on somes exceptions with SSL connection
..


Patch Set 2:

(6 comments)

looks good. I'll have another think about it. Could you ask Sailesh to take a 
look as well, please?

http://gerrit.cloudera.org:8080/#/c/7229/2/be/src/catalog/catalog-service-client-wrapper.h
File be/src/catalog/catalog-service-client-wrapper.h:

Line 25: class CatalogServiceClientWrapper: public CatalogServiceClient {
nit: space before :


http://gerrit.cloudera.org:8080/#/c/7229/2/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:

PS2, Line 207: ||
Should this really be && ?


http://gerrit.cloudera.org:8080/#/c/7229/2/be/src/runtime/client-cache-types.h
File be/src/runtime/client-cache-types.h:

PS2, Line 23: CatalogServiceClientWrapper
replace line 38 with this one.


http://gerrit.cloudera.org:8080/#/c/7229/2/be/src/runtime/client-cache.h
File be/src/runtime/client-cache.h:

PS2, Line 243: bool* retry_is_safe = NULL
let's not do it now, but I think we should get rid of this parameter in favour 
of an indicator that it's ok to retry recv (and have this method call 
RetryRpcRecv() itself). Leave a comment, if you agree?


PS2, Line 319: bool send_done = false;
nit: move inside try block


http://gerrit.cloudera.org:8080/#/c/7229/2/be/src/statestore/statestore-service-client-wrapper.h
File be/src/statestore/statestore-service-client-wrapper.h:

PS2, Line 28:  
space really needed? here and below


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8243d4cac93c453e9396b0e24f41e147c8637b8c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..


Patch Set 3: Code-Review+2

Carry +2, rebase.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#4).

Change subject: IMPALA-4856: Port data stream service to KRPC
..

IMPALA-4856: Port data stream service to KRPC

This patch ports the data-flow parts of ImpalaInternalService to KRPC.

* ImpalaInternalService is split into two services. The first,
  ImpalaInternalService, deals with control messages for plan fragment
  instance execution, cancellation and reporting, and remains
  implemented in Thrift for now. The second, DataStreamService, handles
  large-payload RPCs for transmitting runtime filters and row batches
  between hosts.

* In the DataStreamService, all RPCs use 'native' protobuf. The
  DataStreamService starts on the port previously reserved for the
  StatestoreSubscriberService (which is also a KRPC service), to avoid
  having to configure another port when starting Impala. When the
  ImpalaInternalService is ported to KRPC, all services will run on one
  port.

* To support needing to address two different backend services, a data
  service port has been added to TBackendDescriptor.

* This patch adds support for asynchronous RPCs to the RpcMgr and Rpc
  classes. Previously, Impala used fixed size thread pools + synchronous
  RPCs to achieve some parallelism for 'broadcast' RPCs like filter
  propagation, or a dedicated per-sender+receiver pair thread on the
  sender side in the DataStreamSender case. In this patch, the
  PublishFilter() and TransmitData() RPCs are sent asynchronously using
  KRPC's thread pools.

* The TransmitData() protocol has changed to adapt to asynchronous
  RPCs. The full details are in data-stream-mgr.h.

* As a result, DataStreamSender no longer creates a
  thread-per-connection on the sender side.

* Both tuple transmission and runtime filter publication use sidecars to
  minimise the number of copies and serialization steps required.

* Also include a fix for KUDU-2011 that properly allows sidecars to be
  shared between KRPC and the RPC caller (fixing IMPALA-5093, a
  corruption bug).

* A large portion of this patch is the replacement of TRowBatch with its
  Protobuf equivalent, RowBatchPb. The replacement is a literal port of
  the data structure, and row-batch-test, row-batch-list-test and
  row-batch-serialize-benchmark continue to execute without major logic
  changes.

* Simplify FindRecvr() logic in DataStreamManager. No-longer need to
  handle blocking sender-side, so no need for complex promise-based
  machinery. Instead, all senders with no receiver are added to a
  per-receiver list, which is processed when the receiver arrives. If it
  does not arrive promptly, the DataStreamManager cleans them up after
  FLAGS_datastream_sender_timeout_ms.

* This patch also begins a clean-up of how ImpalaServer instances are
  created (by removing CreateImpalaServer), and clarifying the
  relationship between ExecEnv and ImpalaServer. ImpalaServer now
  follows the standard construct->Init()->Start()->Join() lifecycle that
  we use for other services.

* Ensure that all addresses used for KRPCs are fully resolved, avoiding
  the need to resolve them for each RPC.

TESTING
---

* New tests added to rpc-mgr-test.

TO DO
-

* Re-enable throughput and latency measurements per data-stream sender
  when that information is exposed from KRPC (KUDU-1738).

* TLS and Kerberos are still not supported by KRPC in this patch.

Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b
---
M .clang-format
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/benchmarks/row-batch-serialize-benchmark.cc
M be/src/common/init.cc
M be/src/common/status.cc
M be/src/common/status.h
M be/src/exprs/expr-test.cc
M be/src/kudu/rpc/rpc_sidecar.cc
M be/src/kudu/rpc/rpc_sidecar.h
M be/src/rpc/CMakeLists.txt
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/common.proto
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/rpc/rpc.h
M be/src/runtime/backend-client.h
M be/src/runtime/client-cache-types.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/data-stream-sender.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch-serialize-test.cc
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/scheduling/backend-config-test.cc
M 

[Impala-ASF-CR] IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore services to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#14).

Change subject: IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore 
services to KRPC
..

IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore services to KRPC

This patch adds the core abstractions necessary for using Kudu RPC for
Impala's backend services. The StatestoreService and
StatestoreSubscriberService are both ported to KRPC to demonstrate how
the new RPC framework operates.

The main new class is RpcMgr, which is responsible for both services run
by a daemon and for obtaining clients to remote services. The new Rpc
class helps clients build rpc invocation objects, and use them to call
remote methods.

Also done:
 * Utility code to convert from a Thrift structure to a Protobuf
   wrapper. Thrift RPCs do not need to be ported to Protobuf to work.
 * Added more build support for Protobuf generation. All library targets
   now depend on the Protobuf generation step, just as they do Thrift.
 * thrift-server-test is rewritten to use Beeswax as a test service,
   since the old StatestoreService has been removed.
 * Remove InProcessStatestore that was only used for statestore-test and
   mini-impala-cluster. As far as we know, mini-impala-cluster is unused
   by anyone. (IMPALA-4784)

TESTING:
 * Impala's test suite passes.
 * statestore-test has been rewritten to use new statestore code. It
   includes all test cases from the now-removed Python test_statestore.py
 * rpc-mgr-test unit-tests the RpcMgr and Rpc classes.

TODO:
 * Enable SSL and Kerberos support (already in Kudu's imported
   libraries)

Change-Id: I8dbf12b9ecd71d26d239d31c19b487175194c766
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/codegen/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/catalog-op-executor.cc
M be/src/exec/kudu-util.h
M be/src/experiments/CMakeLists.txt
M be/src/exprs/CMakeLists.txt
M be/src/rpc/CMakeLists.txt
A be/src/rpc/common.proto
A be/src/rpc/rpc-mgr-test.cc
A be/src/rpc/rpc-mgr.cc
A be/src/rpc/rpc-mgr.h
A be/src/rpc/rpc-mgr.inline.h
A be/src/rpc/rpc.h
A be/src/rpc/rpc_test.proto
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/bufferpool/CMakeLists.txt
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/CMakeLists.txt
M be/src/service/CMakeLists.txt
M be/src/service/client-request-state.cc
M be/src/statestore/CMakeLists.txt
D be/src/statestore/statestore-service-client-wrapper.h
D be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
A be/src/statestore/statestore.proto
M be/src/statestore/statestored-main.cc
M be/src/testutil/CMakeLists.txt
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
D be/src/testutil/mini-impala-cluster.cc
M be/src/transport/CMakeLists.txt
M be/src/udf/CMakeLists.txt
M be/src/udf_samples/CMakeLists.txt
M be/src/util/CMakeLists.txt
M be/src/util/collection-metrics.h
M be/src/util/counting-barrier.h
M bin/start-impala-cluster.py
M bin/start-impalad.sh
M common/thrift/CMakeLists.txt
M common/thrift/StatestoreService.thrift
D tests/statestore/test_statestore.py
53 files changed, 1,733 insertions(+), 1,219 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8dbf12b9ecd71d26d239d31c19b487175194c766
Gerrit-PatchSet: 14
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4856: Port data stream service to KRPC
..


Patch Set 5:

(22 comments)

PS4 is a rebase. PS5 includes the review responses (so diff 4->5 if you want to 
see what changed).

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

PS3, Line 106: Ownership is
 :   // shared by the caller, and the RPC subsystem
> Doesn't std::move transfer the ownership so the caller no longer shares the
The shared_ptr is copied in. It's the copy that is then moved into the sidecar 
list.


PS3, Line 143: are owned by the caller
> the ownership is temporarily transferred to the RPC call when this function
I don't think so - the RPC call has pointers, but doesn't have ownership in the 
sense that it has no responsibility for managing a reference count or freeing 
the memory.


PS3, Line 147: 
> Having the names 'func', 'cb' and 'cb_wrapper' all close by each other make
Done


PS3, Line 153: 
> Does this move mean that the params_ member is invalid after this call? If 
Done


PS3, Line 327: 
> Maybe name this 'completion_cb'.
Done


http://gerrit.cloudera.org:8080/#/c/7103/3/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

PS3, Line 389: equest->mutable_bloom_filter()->set_log_heap_space(0);
 : 
request->mutable_bloom_filter()->set_directory_sidecar_idx(-1);
 :   }
> Why wouldn't a move capture ensure the same thing?
proto_filter is a const shared_ptr&. You can't move from it. Instead, we could 
have the argument be shared_ptr, and move from it here; 
they're basically equivalent, it's just a question of where you make the copy.


http://gerrit.cloudera.org:8080/#/c/7103/3/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS3, Line 1207:   VLOG_QUERY << "Not enough memory to allocate filter: "
  :  << PrettyPrinter::Print(heap_space, 
TUnit::BYTES)
  :  << " (query: " << coord->query_id() << ")";
  :   // Disable, as one missing update means a correct filter 
cannot be 
> I would add this to the commit message. This means we would take double the
I don't think so - because params.directory is a sidecar I don't think it's 
been copied since it arrived on the socket. In the Thrift case, the bytestream 
had to be deserialized into a TBloomFilter. That's what's happening here - the 
equivalent 'deserialization' step.

This path should only get taken the first time a filter arrives, and it does 
briefly keep two filters around (the sidecar should get destroyed as soon as 
the RPC is responded to).


http://gerrit.cloudera.org:8080/#/c/7103/3/be/src/runtime/data-stream-recvr.cc
File be/src/runtime/data-stream-recvr.cc:

PS3, Line 280: blocked_senders_.front()
> Is this a right way to dispose a unique_ptr?
Good point - release() is clearer, and get() may have been a benign bug.


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

PS3, Line 58: 
> Not used.
Done


PS3, Line 60: 
> Not used.
Done


PS3, Line 133: scoped_ptr batch_;
> No one really calls GetNumDataBytesSent() (except from our BE test). So, I'
We're gaining correctness - so worth doing (otherwise if someone decides to use 
it in the future, they might run into problems).


PS3, Line 148: 
> A reader of this code might not immediately understand why this class needs
I expanded the comment here.


PS3, Line 170: 
> Why is this set in Init()? Wouldn't it ideally be set it in the constructor
Moved to c'tor.


PS3, Line 175: proto_batch_idx_
> Just want to make sure that this will increase the shared_ptr refcount? It 
Yep - this was a mistake. Removed auto to make it more explicit.


PS3, Line 203: co
> Prefer a more descriptive name "rpc_completion_cb" or something similar.
Done


PS3, Line 214: ck_guard
> channel == nullptr
Done


PS3, Line 252: batch->tuple_data, );
> Is this transferring the ownership to the RPC subsystem ? AddSideCar() inte
The ownership is shared with the batch object. AddSidecar() internally moves 
from the argument, which is a copy (i.e. its own reference).


PS3, Line 266: .release(), rpc_complete_callback);
> This is a subtle change in behavior from previous Impala version. In partic
Any reasonably conservative timeout runs the risk of false negatives if a 
sender is blocked.

I agree with your analysis about this being a change in behaviour. In practice, 
though, here's what I hope will happen: if one write to a node is slow enough 
to previously trigger the timeout, I would expect the statestore RPCs to also 
go slow (and they will time out); the node will be marked as offline and the 
query will be cancelled. 

If there is a situation where this RPC only is slow in writing (but all other 
RPCs to the server are ok), then I agree 

[Impala-ASF-CR] IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore services to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#15).

Change subject: IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore 
services to KRPC
..

IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore services to KRPC

This patch adds the core abstractions necessary for using Kudu RPC for
Impala's backend services. The StatestoreService and
StatestoreSubscriberService are both ported to KRPC to demonstrate how
the new RPC framework operates.

The main new class is RpcMgr, which is responsible for both services run
by a daemon and for obtaining clients to remote services. The new Rpc
class helps clients build rpc invocation objects, and use them to call
remote methods.

Also done:
 * Utility code to convert from a Thrift structure to a Protobuf
   wrapper. Thrift RPCs do not need to be ported to Protobuf to work.
 * Added more build support for Protobuf generation. All library targets
   now depend on the Protobuf generation step, just as they do Thrift.
 * thrift-server-test is rewritten to use Beeswax as a test service,
   since the old StatestoreService has been removed.
 * Remove InProcessStatestore that was only used for statestore-test and
   mini-impala-cluster. As far as we know, mini-impala-cluster is unused
   by anyone. (IMPALA-4784)

TESTING:
 * Impala's test suite passes.
 * statestore-test has been rewritten to use new statestore code. It
   includes all test cases from the now-removed Python test_statestore.py
 * rpc-mgr-test unit-tests the RpcMgr and Rpc classes.

TODO:
 * Enable SSL and Kerberos support (already in Kudu's imported
   libraries)

Change-Id: I8dbf12b9ecd71d26d239d31c19b487175194c766
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/CMakeLists.txt
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/codegen/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/catalog-op-executor.cc
M be/src/exec/kudu-util.h
M be/src/experiments/CMakeLists.txt
M be/src/exprs/CMakeLists.txt
M be/src/rpc/CMakeLists.txt
A be/src/rpc/common.proto
A be/src/rpc/rpc-mgr-test.cc
A be/src/rpc/rpc-mgr.cc
A be/src/rpc/rpc-mgr.h
A be/src/rpc/rpc-mgr.inline.h
A be/src/rpc/rpc.h
A be/src/rpc/rpc_test.proto
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/bufferpool/CMakeLists.txt
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/CMakeLists.txt
M be/src/service/CMakeLists.txt
M be/src/service/client-request-state.cc
M be/src/statestore/CMakeLists.txt
D be/src/statestore/statestore-service-client-wrapper.h
D be/src/statestore/statestore-subscriber-client-wrapper.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
A be/src/statestore/statestore.proto
M be/src/statestore/statestored-main.cc
M be/src/testutil/CMakeLists.txt
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
D be/src/testutil/mini-impala-cluster.cc
M be/src/transport/CMakeLists.txt
M be/src/udf/CMakeLists.txt
M be/src/udf_samples/CMakeLists.txt
M be/src/util/CMakeLists.txt
M be/src/util/collection-metrics.h
M be/src/util/counting-barrier.h
M bin/start-impala-cluster.py
M bin/start-impalad.sh
M common/thrift/CMakeLists.txt
M common/thrift/StatestoreService.thrift
D tests/statestore/test_statestore.py
53 files changed, 1,733 insertions(+), 1,219 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8dbf12b9ecd71d26d239d31c19b487175194c766
Gerrit-PatchSet: 15
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Juan Yu 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#9).

Change subject: IMPALA-4669: [SECURITY] Add security library to build
..

IMPALA-4669: [SECURITY] Add security library to build

* Minor compilation fix
* Set toolchain version to include gflags 2.2.0 and glog 0.3.4-p2

Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/kudu/security/init.cc
M be/src/kudu/security/token_verifier.cc
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M cmake_modules/FindKerberos.cmake
7 files changed, 58 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/5717/9
-- 
To view, visit http://gerrit.cloudera.org:8080/5717
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-4856: Port data stream service to KRPC

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#5).

Change subject: IMPALA-4856: Port data stream service to KRPC
..

IMPALA-4856: Port data stream service to KRPC

This patch ports the data-flow parts of ImpalaInternalService to KRPC.

* ImpalaInternalService is split into two services. The first,
  ImpalaInternalService, deals with control messages for plan fragment
  instance execution, cancellation and reporting, and remains
  implemented in Thrift for now. The second, DataStreamService, handles
  large-payload RPCs for transmitting runtime filters and row batches
  between hosts.

* In the DataStreamService, all RPCs use 'native' protobuf. The
  DataStreamService starts on the port previously reserved for the
  StatestoreSubscriberService (which is also a KRPC service), to avoid
  having to configure another port when starting Impala. When the
  ImpalaInternalService is ported to KRPC, all services will run on one
  port.

* To support needing to address two different backend services, a data
  service port has been added to TBackendDescriptor.

* This patch adds support for asynchronous RPCs to the RpcMgr and Rpc
  classes. Previously, Impala used fixed size thread pools + synchronous
  RPCs to achieve some parallelism for 'broadcast' RPCs like filter
  propagation, or a dedicated per-sender+receiver pair thread on the
  sender side in the DataStreamSender case. In this patch, the
  PublishFilter() and TransmitData() RPCs are sent asynchronously using
  KRPC's thread pools.

* The TransmitData() protocol has changed to adapt to asynchronous
  RPCs. The full details are in data-stream-mgr.h.

* As a result, DataStreamSender no longer creates a
  thread-per-connection on the sender side.

* Both tuple transmission and runtime filter publication use sidecars to
  minimise the number of copies and serialization steps required.

* Also include a fix for KUDU-2011 that properly allows sidecars to be
  shared between KRPC and the RPC caller (fixing IMPALA-5093, a
  corruption bug).

* A large portion of this patch is the replacement of TRowBatch with its
  Protobuf equivalent, RowBatchPb. The replacement is a literal port of
  the data structure, and row-batch-test, row-batch-list-test and
  row-batch-serialize-benchmark continue to execute without major logic
  changes.

* Simplify FindRecvr() logic in DataStreamManager. No-longer need to
  handle blocking sender-side, so no need for complex promise-based
  machinery. Instead, all senders with no receiver are added to a
  per-receiver list, which is processed when the receiver arrives. If it
  does not arrive promptly, the DataStreamManager cleans them up after
  FLAGS_datastream_sender_timeout_ms.

* This patch also begins a clean-up of how ImpalaServer instances are
  created (by removing CreateImpalaServer), and clarifying the
  relationship between ExecEnv and ImpalaServer. ImpalaServer now
  follows the standard construct->Init()->Start()->Join() lifecycle that
  we use for other services.

* Ensure that all addresses used for KRPCs are fully resolved, avoiding
  the need to resolve them for each RPC.

TESTING
---

* New tests added to rpc-mgr-test.

TO DO
-

* Re-enable throughput and latency measurements per data-stream sender
  when that information is exposed from KRPC (KUDU-1738).

* TLS and Kerberos are still not supported by KRPC in this patch.

Change-Id: Ia66704be7a0a8162bb85556d07b583ec756c584b
---
M .clang-format
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/benchmarks/bloom-filter-benchmark.cc
M be/src/benchmarks/row-batch-serialize-benchmark.cc
M be/src/common/init.cc
M be/src/common/status.cc
M be/src/common/status.h
M be/src/exprs/expr-test.cc
M be/src/kudu/rpc/rpc_sidecar.cc
M be/src/kudu/rpc/rpc_sidecar.h
M be/src/rpc/CMakeLists.txt
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/common.proto
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/rpc/rpc.h
M be/src/runtime/backend-client.h
M be/src/runtime/client-cache-types.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/data-stream-sender.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch-serialize-test.cc
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/scheduling/backend-config-test.cc
M 

[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#10).

Change subject: IMPALA-4669: [SECURITY] Add security library to build
..

IMPALA-4669: [SECURITY] Add security library to build

* Minor compilation fix
* Set toolchain version to include gflags 2.2.0 and glog 0.3.4-p2
* Look for krb5 headers in toolchain (but dynamically link against libraries)

Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/kudu/security/init.cc
M be/src/kudu/security/token_verifier.cc
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M cmake_modules/FindKerberos.cmake
7 files changed, 58 insertions(+), 22 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-4669: [KRPC] Add kudu rpc library to build

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new patch set (#10).

Change subject: IMPALA-4669: [KRPC] Add kudu_rpc library to build
..

IMPALA-4669: [KRPC] Add kudu_rpc library to build

Import FindKRPC.cmake from Apache Kudu.

One minor linking change to krpc/CMakeLists.txt.

Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
---
M CMakeLists.txt
M be/CMakeLists.txt
A cmake_modules/FindKRPC.cmake
3 files changed, 129 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33203e95dff07c87a6ec5c7a31b7a583b91849bc
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 

[Impala-ASF-CR] IMPALA-4889: Use client sidecars for Thrift RPCs

[Henry Robinson (Code Review)]

Hello Marcel Kornacker,

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

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

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

Change subject: IMPALA-4889: Use client sidecars for Thrift RPCs
..

IMPALA-4889: Use client sidecars for Thrift RPCs

This patch changes the way Thrift structures are serialized with
KRPC. Instead of serializing them to a byte stream, and then writing
that stream to a Protobuf object which is serialized again en route to
the wire, the Thrift objects are serialized to byte streams which are
then directly written to the wire as a 'sidecar'. This saves a copy and
serialization step both on the sender and receiver sides of the RPC.

Change-Id: I0fcc87c6b67aa167d70ae022663b14bc90261c95
---
M be/src/rpc/common.proto
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc.h
M be/src/rpc/thrift-util.h
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-test.cc
M be/src/statestore/statestore.cc
7 files changed, 135 insertions(+), 26 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0fcc87c6b67aa167d70ae022663b14bc90261c95
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 

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

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Michael Ho, Matthew Jacobs, Tim Armstrong,

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

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

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

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

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

A few miscellaneous changes to allow kudu_util to compile with Impala.

Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.

Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.

Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.

Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.

Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/common/CMakeLists.txt
A be/src/common/kudu_version.cc
M be/src/common/logging.cc
M be/src/exec/kudu-util.h
A be/src/kudu/gutil
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/cache.cc
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/locks.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
D cmake_modules/FindOpenSSL.cmake
18 files changed, 140 insertions(+), 90 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4669: [SECURITY] Add security library to build

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4669: [SECURITY] Add security library to build
..


Patch Set 10:

Michael - this includes the KRB5 and OpenSSL linking fixes.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f327810066bee7f3ac107b0295480fb9ed45e14
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
..


Patch Set 5:

(1 comment)

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

Line 179:   if 
(materialized_row_batches_->AddBatchWithTimeout(move(row_batch), 100)) {
> I added a DCHECK that get hits immediately in the old buggy code with test_
Thanks Tim - you're quite right, and I misunderstood what you're saying. I 
think it's brittle to rely on the fact that move() may or may not result 
ultimately in a moved-from rvalue, but guess it's useful here with no obvious 
better implementation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
..


Patch Set 5:

(1 comment)

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

Line 179:   if 
(materialized_row_batches_->AddBatchWithTimeout(move(row_batch), 100)) {
> Just passing through.
You could add an output parameter to BlockingPutWithTimeout() (rather than 
change the return type). Or the row batch queue should be of shared_ptr<>. I 
don't think we should rely on when move() is called in the blocking queue (and 
I'm not sure that analysis is quite right, as you would need to move() the row 
batch into AddBatchWithTimeout() to make it an rvalue-ref; i.e. before any 
timeout).

Sailesh - Where did you see discouraging using unique_ptr<> as an rvalue-ref? 
The idea of rvalues is that you move() into them, which means you are 
explicitly relinquishing ownership of the moved-from object.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Henry Robinson has abandoned this change.

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..


Abandoned

Bad merge - will restore when I fix it.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Michael Ho, Sailesh Mukil, Tim Armstrong,

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

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

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

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..

IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

Change allocation pattern for Codec objects in RowBatch to be
stack-allocated. Make c'tors and Init() methods of codec implementations
publicly visible in order to do so.

Refactor HdfsParquetWriter etc. to deal properly with Status returned in
all cases.

Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
---
M be/src/exec/hdfs-avro-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-sequence-table-writer.cc
M be/src/exec/parquet-column-stats.cc
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/experiments/compression-test.cc
M be/src/runtime/row-batch.cc
M be/src/util/codec.h
M be/src/util/compress.h
M be/src/util/decompress-test.cc
M be/src/util/decompress.h
12 files changed, 160 insertions(+), 151 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5481: Clarify RowDescriptor ownership

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5481: Clarify RowDescriptor ownership
..


Patch Set 6:

Thanks - I'm in the middle of that :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2fc39170f775581d406b6a97445046f508d8d75f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5481: Clarify RowDescriptor ownership

[Henry Robinson (Code Review)]

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5481: Clarify RowDescriptor ownership
..

IMPALA-5481: Clarify RowDescriptor ownership

RowDescriptors are originally allocated in-line with the exec node that
uses them. Their lifetime is therefore guaranteed to be as long as the
parent fragment instance. However, we copy the RowDescriptors into
RowBatches, which adds allocation pressure (RowDescriptors contain
vectors), and is unnecessary as a) the descriptor is constant and b)
RowBatches get destroyed before exec nodes.

This patch standardises ownership of RowDescriptor objects, by changing
members that were copies or const references to RowDescriptors to be
const RowDescriptor*. Method arguments are either const* to convey that
ownership is to be shared, or const& to convey that the descriptor is to
be used but not mutated by the callee.

The tradeoff of fewer allocations appears to outweigh any loss of cache
locality due to sharing the RowDescriptor. On a 16-node cluster that
previously spend ~20% of its tcmalloc time allocating RowDescriptors,
this patch reduced that time to 0%.

Change-Id: I2fc39170f775581d406b6a97445046f508d8d75f
---
M be/src/benchmarks/row-batch-serialize-benchmark.cc
M be/src/exec/aggregation-node.cc
M be/src/exec/analytic-eval-node.cc
M be/src/exec/blocking-join-node.cc
M be/src/exec/data-sink.cc
M be/src/exec/data-sink.h
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hash-join-node.cc
M be/src/exec/hbase-table-sink.cc
M be/src/exec/hbase-table-sink.h
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/exec/kudu-scanner.cc
M be/src/exec/kudu-table-sink.cc
M be/src/exec/kudu-table-sink.h
M be/src/exec/nested-loop-join-builder.cc
M be/src/exec/nested-loop-join-builder.h
M be/src/exec/nested-loop-join-node.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/exec/row-batch-cache.h
M be/src/exec/row-batch-list-test.cc
M be/src/exec/sort-node.cc
M be/src/exec/topn-node.cc
M be/src/exec/union-node.cc
M be/src/exec/unnest-node.cc
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream-v2-test.cc
M be/src/runtime/buffered-tuple-stream-v2.cc
M be/src/runtime/buffered-tuple-stream-v2.h
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-mgr.h
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-recvr.h
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/data-stream-sender.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M be/src/runtime/row-batch-serialize-test.cc
M be/src/runtime/row-batch-test.cc
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/sorted-run-merger.cc
M be/src/runtime/sorted-run-merger.h
M be/src/runtime/sorter.cc
M be/src/util/debug-util.cc
57 files changed, 330 insertions(+), 326 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2fc39170f775581d406b6a97445046f508d8d75f
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5537: Retry RpcRecv on recv timeout exception for SSL connection

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5537: Retry RpcRecv on recv timeout exception for SSL 
connection
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7229/1/be/src/rpc/thrift-util.cc
File be/src/rpc/thrift-util.cc:

PS1, Line 209: Called write on non-open socket"
> It seems that we still didn't handle the case of send failure for failed co
I agree, that would be safer - then we can be certain what phase the exceptions 
get thrown in.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8243d4cac93c453e9396b0e24f41e147c8637b8c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..

IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

Change allocation pattern for Codec objects in RowBatch to be
stack-allocated. Make c'tors and Init() methods of codec implementations
publicly visible in order to do so.

Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
---
M be/src/runtime/row-batch.cc
M be/src/util/codec.h
M be/src/util/compress.h
M be/src/util/decompress.h
4 files changed, 57 insertions(+), 77 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-5481: Clarify RowDescriptor ownership

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5481: Clarify RowDescriptor ownership
..


Patch Set 7: Code-Review+1

Rebase, carry +1.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2fc39170f775581d406b6a97445046f508d8d75f
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Hello Sailesh Mukil, Tim Armstrong,

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

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

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

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..

IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

Change allocation pattern for Codec objects in RowBatch to be
stack-allocated. Make c'tors and Init() methods of codec implementations
publicly visible in order to do so.

Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
---
M be/src/runtime/row-batch.cc
M be/src/util/codec.h
M be/src/util/compress.h
M be/src/util/decompress.h
4 files changed, 93 insertions(+), 104 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7226/1/be/src/runtime/row-batch.cc
File be/src/runtime/row-batch.cc:

PS1, Line 100: DCHECK(input_batch.compression_type == THdfsCompression::LZ4)
> DCHECK_EQ();
Done


Line 226: VLOG_ROW << "uncompressed size: " << size << ", compressed size: 
" << compressed_size;
> Not your change either but it's unfortunate that we don't call Close() if P
Done - used a ScopedExitTrigger to make Close() is always called.


http://gerrit.cloudera.org:8080/#/c/7226/1/be/src/util/compress.h
File be/src/util/compress.h:

Line 47:   virtual Status Init();
> WARN_UNUSED_RESULT
Done


http://gerrit.cloudera.org:8080/#/c/7226/1/be/src/util/decompress.h
File be/src/util/decompress.h:

PS1, Line 38: ;
> WARN_UNUSED_RESULT. Same below.
Done


Line 41:   const uint8_t* input, int64_t* output_length, uint8_t** output);
> WARN_UNUSED_RESULT. Same below.
Done


Line 44:   bool* stream_end);
> WARN_UNUSED_RESULT. Same below.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4b5a8d2cc315db50e5d70b1191702206de3450d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5536: Fix TCLIService thrift compilation on Hive 2

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5536: Fix TCLIService thrift compilation on Hive 2
..


Patch Set 2:

(1 comment)

Joe - is this rebased on the Sentry version revert patch? If not, I think this 
might fail GVO.

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

Line 2812:   Pair statsTaskParam =
nit: long line


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e1477993ee3ccddd236609efec7bb23f20a7b66
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5532: Stack-allocate compressors in RowBatch (de)serialization

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5532: Stack-allocate compressors in RowBatch 
(de)serialization
..


Patch Set 4:

I fixed the placement of WARN_UNUSED_RESULT in the previous patchset, and also 
changed a method signature in the parquet writer so that the status of a 
compression operation could be checked. Please take another quick look at 
hdfs-parquet-table-writer.cc!

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

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