[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 5:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/878/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
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-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7445/4/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:

PS4, Line 60: If use_statestore is set, a connection to the statestore
:   /// is established.
> delete
Done


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

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

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Hello Michael Ho, Sailesh Mukil,

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

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

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

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..

IMPALA-5670: Misc. tidying of ExecEnv

* Remove duplicated c'tor code for ExecEnv() and delegate it onto
  6-argument c'tor.
* Make some RM-related flags hidden
* Remove unused and untested 'use_statestore' branch in ExecEnv which
  created a scheduler with static hosts rather than those sent by the
  statestore.
* Remove corresponding scheduler c'tor to handle the above path.
* Remove unused 'use_statestore' parameter from
  InProcessImpalaServer::StartAsBackendOnly() and
  InProcessImpalaServer::StartWithClientServers().

Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
---
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/scheduling/scheduler.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/testutil/mini-impala-cluster.cc
M docs/topics/impala_config_options.xml
M docs/topics/impala_upgrading.xml
12 files changed, 71 insertions(+), 178 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7445/4/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:

PS4, Line 60: If use_statestore is set, a connection to the statestore
:   /// is established.
delete


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

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

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 2: Code-Review+2

Included patch for IMPALA-5670 broke some tests. Rebased to remove it, carry +2.

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

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

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/877/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 2: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/874/

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

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

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Hello Sailesh Mukil,

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

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

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

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..

IMPALA-5670: Misc. tidying of ExecEnv

* Remove duplicated c'tor code for ExecEnv() and delegate it onto
  6-argument c'tor.
* Make some RM-related flags hidden
* Remove unused and untested 'use_statestore' branch in ExecEnv which
  created a scheduler with static hosts rather than those sent by the
  statestore.
* Remove corresponding scheduler c'tor to handle the above path.
* Remove unused 'use_statestore' parameter from
  InProcessImpalaServer::StartAsBackendOnly() and
  InProcessImpalaServer::StartWithClientServers().

Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.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/testutil/mini-impala-cluster.cc
M docs/topics/impala_config_options.xml
M docs/topics/impala_upgrading.xml
10 files changed, 63 insertions(+), 173 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7445/3/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:

PS3, Line 62: bool use_statestore
> remove
Done


http://gerrit.cloudera.org:8080/#/c/7445/3/be/src/testutil/mini-impala-cluster.cc
File be/src/testutil/mini-impala-cluster.cc:

PS3, Line 41: DECLARE_string(principal);
> Not used ?
I have another patch that's going to remove the mini-impala-cluster altogether, 
so probably not worth the effort to clean it up!


PS3, Line 81: FLAGS_use_statestore)
> Missing removal ?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
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-HasComments: Yes

[Impala-ASF-CR] Bump Kudu version to f144d57

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] Bump Kudu version to f144d57

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged.

Change subject: Bump Kudu version to f144d57
..


Bump Kudu version to f144d57

Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Reviewed-on: http://gerrit.cloudera.org:8080/7451
Reviewed-by: Matthew Jacobs 
Reviewed-by: Thomas Tauber-Marshall 
Tested-by: Impala Public Jenkins
---
M bin/impala-config.sh
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Matthew Jacobs: Looks good to me, approved
  Thomas Tauber-Marshall: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 

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

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

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


Patch Set 24:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/876/

-- 
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: 24
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-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 (#24).

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


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/5715/24
-- 
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: 24
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-5659: Begin standardizing treatment of thirdparty libraries

[Henry Robinson (Code Review)]

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


IMPALA-5659: Begin standardizing treatment of thirdparty libraries

If Impala was built with --build_shared_libs, some thirdparty libraries
were still statically linked; this could cause runtime errors if the
libraries were also linked into a .so. This patch fixes that issue (for
gflags, glog and protobuf at least) by ensuring that build_shared_libs
is respected for those libraries.

* Standardize thirdparty library handling w/CMake by adding
  IMPALA_ADD_THIRDPARTY_LIB. This creates a symbolic name for each
  library, allowing us to switch the underlying library
  files (e.g. change from static to dynamic linking) without having to
  individually change the link clauses for each target.

* Remove most cases of add_library() from cmake_modules/* - that is all
  handled by IMPALA_ADD_THIRDPARTY_LIB.

* Add shared library detection for a couple of thirdparty
  dependencies (many only detect static libraries), just to prove the concept.

* All thirdparty libraries now print a standard set of messages. For example:

-- --> Adding thirdparty library protoc. <--
-- Header files: 
/data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/include
-- Added shared library dependency protoc: 
/data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/lib/libprotoc.so
-- --> Adding thirdparty library libev. <--
-- Header files: /data/henry/src/cloudera/impala-toolchain/libev-4.20/include
-- Added shared library dependency libev: 
/data/henry/src/cloudera/impala-toolchain/libev-4.20/lib/libev.so

* Some libraries don't quite fit this pattern (LLVM and Boost) - leave
  them as is for now.

* Remove FindOpenSSL.cmake - the toolchain one is more modern.

Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
Reviewed-on: http://gerrit.cloudera.org:8080/7418
Tested-by: Impala Public Jenkins
Reviewed-by: Tim Armstrong 
---
M CMakeLists.txt
M be/CMakeLists.txt
M cmake_modules/FindAvro.cmake
M cmake_modules/FindBreakpad.cmake
M cmake_modules/FindFlatBuffers.cmake
M cmake_modules/FindGFlags.cmake
M cmake_modules/FindGLog.cmake
M cmake_modules/FindGTest.cmake
M cmake_modules/FindHDFS.cmake
M cmake_modules/FindLdap.cmake
M cmake_modules/FindLz4.cmake
D cmake_modules/FindOpenSSL.cmake
M cmake_modules/FindRe2.cmake
M cmake_modules/FindSasl.cmake
M cmake_modules/FindSnappy.cmake
M cmake_modules/FindThrift.cmake
M cmake_modules/FindZlib.cmake
M cmake_modules/kudu_cmake_fns.txt
18 files changed, 155 insertions(+), 301 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
Gerrit-PatchSet: 7
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: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/875/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 3:

(3 comments)

Thanks for cleaning things up. Looks much better.

http://gerrit.cloudera.org:8080/#/c/7445/3/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:

PS3, Line 62: bool use_statestore
remove


http://gerrit.cloudera.org:8080/#/c/7445/3/be/src/testutil/mini-impala-cluster.cc
File be/src/testutil/mini-impala-cluster.cc:

PS3, Line 41: DECLARE_string(principal);
Not used ?


PS3, Line 81: FLAGS_use_statestore)
Missing removal ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
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-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/872/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 5: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/871/

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

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

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 1: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/869/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 2:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/874/

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

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

[Impala-ASF-CR] IMPALA-5676: avoid expensive consistency checks in BTSv2

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded a new change for review.

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

Change subject: IMPALA-5676: avoid expensive consistency checks in BTSv2
..

IMPALA-5676: avoid expensive consistency checks in BTSv2

Doing an O(n) consistency check every time the read or write
page was advanced results in O(n^2) overall runtime.

The fix is to separate the O(1) and O(n) checks and only do the
O(n) checks if:
* The function does an an O(n) pass over the pages anyway (e.g.
  PinStream())
* The function is called only once per read or write pass over the
  stream.

This should make the cost of the checks O(n) (if we make the reasonable
assumption that PrepareForWrite(), PrepareForRead(), PinStream() and
UnpinStream() are called a bounded number of times per stream).

Testing:
Ran BufferedTupleStreamV2Test.

Change-Id: I8b380fcd0568cb73b36a490954bcd316db969ede
---
M be/src/runtime/buffered-tuple-stream-v2.cc
M be/src/runtime/buffered-tuple-stream-v2.h
2 files changed, 46 insertions(+), 28 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 2: Code-Review+2

Oops I mean to +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
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-4674: add new message to stress test

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: add new message to stress test
..


Patch Set 1:

Dan Hecht already reviewed this as part of the big patch 
https://gerrit.cloudera.org/#/c/5801/, but I wanted to pull it out since it's a 
logically independent.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4674: add new message to stress test

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded a new change for review.

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

Change subject: IMPALA-4674: add new message to stress test
..

IMPALA-4674: add new message to stress test

The main IMPALA-4674 commit adds a new OOM failure mode where the query
can't get its minimum reservation during query startup. The new message
includes the string "failed to get minimum memory reservation" along
with some additional context.

Testing:
Ran a stress test using the modified script. Verified it treats failure
to get minimum reservation in the same way as mem limit exceeded.

Change-Id: I1f5e227084dfd50369a9908975305fa5e571c8a8
---
M tests/stress/concurrent_select.py
1 file changed, 1 insertion(+), 0 deletions(-)


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

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

[Impala-ASF-CR] Bump Kudu version to f144d57

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/873/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] Bump Kudu version to f144d57

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1: Code-Review+2

The GVO failure is definitely unrelated, as the test has nothing to do with 
Kudu. I filed: IMPALA-5678

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4925: Cancel finstance if query has finished
..


Patch Set 2:

Thanks for the review - any other comments?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
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] [DOCS] Update list of reserved words

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: [DOCS] Update list of reserved words
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If4a340a033ff3f529061e48c4a5558b1ad1637ef
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4905: Don't send empty insert status to coordinator

[Henry Robinson (Code Review)]

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-4905: Don't send empty insert status to coordinator
..

IMPALA-4905: Don't send empty insert status to coordinator

If the coordinator, in UpdateBackendExecStatus(), receives a report that
includes a TInsertExecStatus, it will call UpdateInsertExecStatus()
which takes the coordinator-wide lock. Avoid doing this for fragment
instances that would only send an empty TInsertExecStatus (including
instances that belong to SELECT queries, not DML queries).

Future changes should fix the locking around the
UpdateBackendExecStatus() path to remove dependencies on
Coordinator::lock_, but this fix is simple and addresses one point of
needless contention.

Change-Id: I314dd8d96922d273c6329266970d249ec8c5c608
---
M be/src/runtime/query-state.cc
1 file changed, 10 insertions(+), 9 deletions(-)


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

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

[Impala-ASF-CR] Bump Kudu version to f144d57

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/868/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5625: write profile when query times out

[Matthew Mulder (Code Review)]

Matthew Mulder has uploaded a new patch set (#2).

Change subject: IMPALA-5625: write profile when query times out
..

IMPALA-5625: write profile when query times out

This change writes query profiles as text files for all of the major
query failure reasons in the concurrent_select stress test.

1) Change the --result-hash-log-dir command-line option to --results-dir
   and update the help text.
2) Introduce two new directories under the directory given by the
   --results-dir command-line argument:
 profiles
 result_hashes
3) Move results into the result_hashes directory.
4) Write the query profile to the profiles directory when a query times
   out or gets an error or incorrect results.
5) Remove the query profile from the log output for unexpected mem
   limit exceeded exceptions. Instead, write those to the the profiles
   directory as well.

Testing:
Ran the stress test with a driver that changes the hashes of some of the
query results in the runtime info json file to inject incorrect result
failures. Set tight bounds on the mem limit and timeout to ensure there
would be timeouts and exceeded memory limit failures. Restarted the
NameNode mid test run to induce a query failure. That covers the 4 cases
for which an exception is thrown and profile is written for query
failures. Verified that the profiles were written for each kind of
query failure.

Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
---
M tests/stress/concurrent_select.py
1 file changed, 90 insertions(+), 60 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1dbdf5fcf97d6c5681c9fc8fb9eb448bc459b3b0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Mulder 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Wood 

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

(2 comments)

> I think we should do this for MemLimitExceeded() too:
 > 
 > Joe pointed out on IMPALA-5598 that a lot of time was being spent
 > there, causing serious problems during the stress test.

What change would you suggest? Should I modify Status::MemLimitExceeded to not 
print the stacktrace?

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

PS1, Line 664: TExecSummary exec_summary;
> All the data would be copied over if you're not using a reference. Ideally 
a copy is required here so it can be used after releasing the query log lock


PS1, Line 680: *result = exec_summary;
> This is pointing to a local stack variable, which will be an invalid refere
this will copy the exec_summary object into *result


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

(2 comments)

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

PS1, Line 664: TExecSummary exec_summary;
> All the data would be copied over if you're not using a reference. Ideally 
nvm, I didn't realize that you do need a copy since query_record could go out 
of scope. ignore.


PS1, Line 680: *result = exec_summary;
> This is pointing to a local stack variable, which will be an invalid refere
ignore.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Hello Sailesh Mukil,

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

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

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

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..

IMPALA-5670: Misc. tidying of ExecEnv

* Remove duplicated c'tor code for ExecEnv() and delegate it onto
  6-argument c'tor.
* Make some RM-related flags hidden
* Remove unused and untested 'use_statestore' branch in ExecEnv which
  created a scheduler with static hosts rather than those sent by the
  statestore.
* Remove corresponding scheduler c'tor to handle the above path.
* Remove unused 'use_statestore' parameter from
  InProcessImpalaServer::StartAsBackendOnly().

Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
---
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.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/testutil/mini-impala-cluster.cc
M docs/topics/impala_config_options.xml
M docs/topics/impala_upgrading.xml
10 files changed, 58 insertions(+), 165 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
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 

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7445/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

PS2, Line 67: use_statestore
> Can you please also remove DECLARE_int32(use_statestore) in impalad-main.cc
Done and done.


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

PS2, Line 168: impala_server_
> initialized to nullptr ?
Done


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

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

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/872/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7445/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

PS2, Line 67: use_statestore
Can you please also remove DECLARE_int32(use_statestore) in impalad-main.cc ?

There are few other references to use_statestore in testutil. Not sure if you 
wanna clean them up too ?


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

PS2, Line 168: impala_server_
initialized to nullptr ?


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

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

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

(2 comments)

This should be a good improvement! Just have a couple of comments.

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

PS1, Line 664: TExecSummary exec_summary;
All the data would be copied over if you're not using a reference. Ideally 
you'd want this to be a const ref, however, you can't leave a const ref 
uninitialized.

So why not have the 'query_record' iterator in the outer scope here and get rid 
of these new local variables you created.


PS1, Line 680: *result = exec_summary;
This is pointing to a local stack variable, which will be an invalid reference 
after the function returns.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5539: Fix Kudu timestamp with -use local tz for unix ts

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5539: Fix Kudu timestamp with -use_local_tz_for_unix_ts
..


Patch Set 4:

(2 comments)

LGTM aside from expanding the matrix of query options

http://gerrit.cloudera.org:8080/#/c/7311/4/tests/custom_cluster/test_local_tz_conversion.py
File tests/custom_cluster/test_local_tz_conversion.py:

Line 36: self.run_test_case('QueryTest/utc-timestamp-functions', vector)
We should make sure to run this with the same test matrix as test_exprs - i.e. 
with codegen enabled and disable and with expr rewrites enabled and disabled.


http://gerrit.cloudera.org:8080/#/c/7311/4/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:

Line 150: class TestUtcTimestampFunctions(ImpalaTestSuite):
Should also run this with the same test dimensions as TestExprs


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6898/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:

PS2, Line 60: 
> is this needed?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

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

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..

IMPALA-5167: Reduce the number of Kudu clients created (FE)

Creating Kudu clients is very expensive as each will fetch
metadata from the Kudu master, so we should minimize the
number of Kudu clients that get created.

This patch stores a map from Kudu master addressed to Kudu
clients in KuduUtil to be used across the FE and catalog.
Another patch has already addressed the BE.

This relies on two changes on the Kudu side: one that clears
non-covered range entries from the client's cache on table
open (d07ecd6ded01201c912d2e336611a6a941f48d98), and one
that automatically refreshes auth tokens when they expire
(603c1578c78c0377ffafdd9c427ebfd8a206bda3).

Testing:
- Ran a stress test on a 10 node cluster: scan of a small
  Kudu table, 1000 concurrent queries, load on the Kudu
  master was reduced signficantly, from ~50% cpu to ~5%.
  (with the BE changes included)
- Ran the Kudu e2e tests.
- Manually ran a test with concurrent INSERTs and
  'ALTER TABLE ADD PARTITION' (which is affected by the
  Kudu side change mentiond above) and verified
  correctness.

Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
---
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
4 files changed, 41 insertions(+), 21 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5670: Misc. tidying of ExecEnv

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5670: Misc. tidying of ExecEnv
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7445/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

Line 135: // TODO: Need refactor to get rid of duplicated code.
Will remove this line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3886bc0edfae0c65c302e1a1812632e60668cd4c
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-5275: Avoid printing status stack trace on hot paths

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

I think we should do this for MemLimitExceeded() too:

Joe pointed out on IMPALA-5598 that a lot of time was being spent there, 
causing serious problems during the stress test.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5511: Add process start time to debug web page

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5511: Add process start time to debug web page
..


Patch Set 6:

(6 comments)

I think the code would be cleaner if the metric always had the same name - do 
you feel strongly about keeping them separate for different processes? We can 
also probably keep the changes out of process-state-info with a little 
refactoring. Let me know what you think.

http://gerrit.cloudera.org:8080/#/c/7363/6/be/src/util/metrics.cc
File be/src/util/metrics.cc:

PS6, Line 224: getMetricByName
GetMetricByName


PS6, Line 224: std::
remove std:: in files.


PS6, Line 227: NULL
prefer nullptr now that we're using C++11.


http://gerrit.cloudera.org:8080/#/c/7363/6/be/src/util/process-state-info.cc
File be/src/util/process-state-info.cc:

PS6, Line 191:   StringProperty* start_time_metric = GetMetricFromChildGroup(
 :   "impala-server", "impala-server.start-time");
 :   if (start_time_metric == NULL) {
 : start_time_metric = GetMetricFromChildGroup(
 :   "catalog", "catalog-server.start-time");
 :   }
 :   if (start_time_metric == NULL) {
 : start_time_metric = GetMetricFromChildGroup(
 :   "statestore", "statestore.start-time");
 :   }
 : 
 :   return start_time_metric;
I think it's better to avoid this complication, and just have 
"process-start-time" for all processes.


PS6, Line 292: << "  Process Start Time: " << endl
 :  << "" << GetString("start_time") << endl;
I think it would be better to have the process start time be a json property 
that was added to the document produced by the webserver. That way we can 
choose how to present the start time. I think it would simplify this patch a 
lot as well, because we could probably do without all the changes in this file 
- we can just add that logic to webserver::RootHandler() itself.


http://gerrit.cloudera.org:8080/#/c/7363/6/be/src/util/webserver.cc
File be/src/util/webserver.cc:

PS6, Line 163: metrics_(NULL) {
I suggest a small refactor to avoid adding this dependency on the metrics group 
to the webserver: move the RootHandler() method to the default-path-handlers.cc 
file, and have it added as part of AddDefaultUrlCallbacks(). That method 
already takes a MetricGroup, which you can bind to the RootHandler callback 
(let me know if you need help with bit of C++ trickery - it's basically:

   void RootHandler(MetricGroup* metrics, const ArgumentMap& args, Document* 
doc) {

  }

  // In AddDefaultPathHandlers - make a wrapper fn that calls RootHandler() 
with the metric group.
  webserver->RegisterUrlCallback([metrics](const ArgumentMap& args, Document* 
doc) { RootHandler(metrics, args, doc); });


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05ae2f80835b1b0e4bc3b38731778ba0871338a4
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6898/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:

PS2, Line 60: import com.google.common.collect.Sets;
is this needed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 5:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/871/

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

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

[Impala-ASF-CR] IMPALA-5275: Avoid printing status stack trace on hot paths

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5275: Avoid printing status stack trace on hot paths
..


Patch Set 1:

(2 comments)

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

PS1, Line 663: , is_query_missing =false;
separate line, space after =


PS1, Line 668: is_query_missing = query_record == query_log_index_.end()
Put this on a separate line for clarity/readability.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief083f558fba587381aa7fe8f99da279da02f1f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4276: Profile displays non-default query options set by planner

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4276: Profile displays non-default query options set by 
planner
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I08e9dc2bebb83101976bbbd903ee48c5068dbaab
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


Patch Set 6:

Fix a bug in the way HDFS libraries were linked for libfesupport.so (must 
always be shared, patch made them static in certain circumstances).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


Patch Set 6:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/870/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Henry Robinson (Code Review)]

Hello Impala Public Jenkins, Michael Ho, Tim Armstrong,

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

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

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

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..

IMPALA-5659: Begin standardizing treatment of thirdparty libraries

If Impala was built with --build_shared_libs, some thirdparty libraries
were still statically linked; this could cause runtime errors if the
libraries were also linked into a .so. This patch fixes that issue (for
gflags, glog and protobuf at least) by ensuring that build_shared_libs
is respected for those libraries.

* Standardize thirdparty library handling w/CMake by adding
  IMPALA_ADD_THIRDPARTY_LIB. This creates a symbolic name for each
  library, allowing us to switch the underlying library
  files (e.g. change from static to dynamic linking) without having to
  individually change the link clauses for each target.

* Remove most cases of add_library() from cmake_modules/* - that is all
  handled by IMPALA_ADD_THIRDPARTY_LIB.

* Add shared library detection for a couple of thirdparty
  dependencies (many only detect static libraries), just to prove the concept.

* All thirdparty libraries now print a standard set of messages. For example:

-- --> Adding thirdparty library protoc. <--
-- Header files: 
/data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/include
-- Added shared library dependency protoc: 
/data/henry/src/cloudera/impala-toolchain/protobuf-2.6.1/lib/libprotoc.so
-- --> Adding thirdparty library libev. <--
-- Header files: /data/henry/src/cloudera/impala-toolchain/libev-4.20/include
-- Added shared library dependency libev: 
/data/henry/src/cloudera/impala-toolchain/libev-4.20/lib/libev.so

* Some libraries don't quite fit this pattern (LLVM and Boost) - leave
  them as is for now.

* Remove FindOpenSSL.cmake - the toolchain one is more modern.

Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
---
M CMakeLists.txt
M be/CMakeLists.txt
M cmake_modules/FindAvro.cmake
M cmake_modules/FindBreakpad.cmake
M cmake_modules/FindFlatBuffers.cmake
M cmake_modules/FindGFlags.cmake
M cmake_modules/FindGLog.cmake
M cmake_modules/FindGTest.cmake
M cmake_modules/FindHDFS.cmake
M cmake_modules/FindLdap.cmake
M cmake_modules/FindLz4.cmake
D cmake_modules/FindOpenSSL.cmake
M cmake_modules/FindRe2.cmake
M cmake_modules/FindSasl.cmake
M cmake_modules/FindSnappy.cmake
M cmake_modules/FindThrift.cmake
M cmake_modules/FindZlib.cmake
M cmake_modules/kudu_cmake_fns.txt
18 files changed, 155 insertions(+), 301 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/869/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

[Matthew Jacobs (Code Review)]

Matthew Jacobs has uploaded a new change for review.

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

Change subject: IMPALA-5638: Fix Kudu table set tblproperties inconsistencies
..

IMPALA-5638: Fix Kudu table set tblproperties inconsistencies

Kudu tables did not treat some table properties correctly.

Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke_kudu.test
M tests/authorization/test_grant_revoke.py
5 files changed, 165 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Attila Jeges (Code Review)]

Attila Jeges has uploaded a new patch set (#3).

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..

IMPALA-5407: Fix crash in HdfsSequenceTableWriter

The following use of sequence file writer can lead to a crash:
> set compression_codec=gzip;
> set seq_compression_mode=record;
> set allow_unsupported_formats=1;
> create table seq_tbl like tbl stored as sequencefile;
> insert into seq_tbl select * from tbl;

This fix removes the MemPool::FreeAll() call from
HdfsSequenceTableWriter::Flush(). Freeing the memory pool in Flush()
is incorrect because a memory pool buffer is cached by the compressor
in the table writer which isn't reset across calls to Flush().

If the file that is being written is big enough,
HdfsSequenceTableWriter::AppendRows() will call Flush() multiple
times causing memory corruption.

Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
---
M be/src/exec/hdfs-sequence-table-writer.cc
M testdata/workloads/functional-query/queries/QueryTest/seq-writer.test
2 files changed, 18 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Attila Jeges (Code Review)]

Attila Jeges has posted comments on this change.

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..


Patch Set 2:

(2 comments)

Added a test case to seq-writer.test and made sure that it crashes impalad if 
the fix is not applied.

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

PS2, Line 16: The issue is that HdfsSequenceTableWriter::Flush() frees the 
per-file
: memory pool
> Please clarify that HdfsSequenceTableWriter::AppendRows() may call Flush() 
Done


PS2, Line 17: in use
> cached in the compressor
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Michael Ho 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5407: Fix crash in HdfsSequenceTableWriter

[Attila Jeges (Code Review)]

Attila Jeges has uploaded a new patch set (#3).

Change subject: IMPALA-5407: Fix crash in HdfsSequenceTableWriter
..

IMPALA-5407: Fix crash in HdfsSequenceTableWriter

The following use of sequence file writer can lead to a crash:
> set compression_codec=gzip;
> set seq_compression_mode=record;
> set allow_unsupported_formats=1;
> create table seq_tbl like tbl stored as sequencefile;
> insert into seq_tbl select * from tbl;

This fix removes the MemPool::FreeAll() call from
HdfsSequenceTableWriter::Flush(). Freeing the memory pool in Flush()
is incorrect because a memory pool buffer is cached by the compressor
in the table writer which isn't reset across calls to Flush().

If the file that is being written is big enough,
HdfsSequenceTableWriter::AppendRows() will call Flush() multiple
times causing memory corruption.

Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
---
M be/src/exec/hdfs-sequence-table-writer.cc
M testdata/workloads/functional-query/queries/QueryTest/seq-writer.test
2 files changed, 18 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] [DOCS] Update list of reserved words

[John Russell (Code Review)]

John Russell has uploaded a new change for review.

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

Change subject: [DOCS] Update list of reserved words
..

[DOCS] Update list of reserved words

For 2.9, I believe the only new reserved
keyword is TABLESAMPLE from IMPALA-5309.

Based on commit history from:
https://github.com/apache/incubator-impala/commits/master/fe/src/main/jflex/sql-scanner.flex

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


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If4a340a033ff3f529061e48c4a5558b1ad1637ef
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 6:

(4 comments)

None of these comments are must-dos, just things I've noticed.

http://gerrit.cloudera.org:8080/#/c/7332/6/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS6, Line 104:   db_name = "grant_rev_db_" + get_random_id(5)
Why use a random suffix here but not in test_grant_revoke?


PS6, Line 105:   db_name_upper_case = "XDB_" + get_random_id(5)
 :   db_name_mixed_case = "YdB_" + get_random_id(5)
Maybe use TEST_GRANT_REVOKE and Test_GraNt_revoke or similar for prefixes?


Line 138:   format(role_name))
> nit: Single line wherever applicable.
Yeah, most of these fit within 90 chars on 1 line. For the ones that don't, you 
can use this style, which is PEP-008 compliant:

  self.client.execute(
  "grant select on table {0}.test1 to {1}".format(db_name, role_name))


PS6, Line 195: result = self.client.execute("show tables in functional");
Not your change, but could you remove this semi-colon?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 6: Code-Review+1

(1 comment)

Not sure if can +2 this given the change isn't trivial, so +1. Change LGTM.

http://gerrit.cloudera.org:8080/#/c/7332/6/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

Line 138:   format(role_name))
nit: Single line wherever applicable.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Bump Kudu version to f144d57

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/868/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] Bump Kudu version to f144d57

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Bump Kudu version to f144d57
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change.

Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h
File be/src/gutil/hash/hash.h:

Line 222
> I think you might be right. Where was it used before your patch? In the has
Yes, it seems the specializations for hash are only used in the hash_* 
container.

e.g.
00082   template,
00083class _EqualKey = equal_to<_Key>, class _Alloc = allocator<_Tp> >
00084 class hash_map

By the way, I have not found any use case to create an object for hash_* 
container, so the code is now unused. Do you think we should leave it to use it 
later? Or it is better to remove the code to improve code coverage?


Line 278
> My suggestion is to dig into this library and understand the details of wha
I understand what you worry about. Let me look into the code more deeply. 

By the way, did you check my comment in the previous patch set? It would be 
nice if you take a look at the comment: 
https://gerrit.cloudera.org/#/c/7414/3/be/src/gutil/hash/hash.h@a251


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5529: Add additional function signatures for TRUNC()

[Kim Jin Chul (Code Review)]

Kim Jin Chul has posted comments on this change.

Change subject: IMPALA-5529: Add additional function signatures for TRUNC()
..


Patch Set 1:

(4 comments)

Hi,

Would you please review my code with comments? Thanks.
I would like to add some unit tests into 
java/org/apache/impala/analysis/AnalyzeExprsTest.java. Do you think this is a 
right place?
Please let me know the location if you have E2E test set.

Best regards,
Jinchul

http://gerrit.cloudera.org:8080/#/c/7450/1/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:

PS1, Line 278:   [['trunc'], 'BIGINT', ['BIGINT'], ''],
 :   [['trunc'], 'INT', ['INT'], ''],
 :   [['trunc'], 'SMALLINT', ['SMALLINT'], ''],
 :   [['trunc'], 'TINYINT', ['TINYINT'], ''],
1) I believe there is nothing to do. It just returns an input value.
I think we don't have to call backend function such as Truncate/Round, so I 
would like to do nothing here as coalesce's implementation. By the way, my 
implementation seems to be insufficient because backend throws "Function trunc 
is not implemented". Would you please let me know if there is any special 
handling?

2) What do you think about the specified types such as a series of INT type? I 
guess we need to support more premitive types to work the function properly.


PS1, Line 282: 'BIGINT', ['DOUBLE']
In the description of the JIRA, the reporter wrote that double precision of the 
argument should be double precision, but I am wondering it is expected behavior 
because of "DECIMAL(*, *) = TRUNC(DECIMAL(*,*) [, *INT])" which means return 
type should be same of the first argument type.

If the return type should be DOUBLE, I guess we need to add implicit conversion.


PS1, Line 284: 'DOUBLE', ['DOUBLE', 'INT']
1) I think "DOUBLE = TRUNC(DOUBLE, *INT)" is required. Do you agree on this?

2) The author before my patch intended implicit type conversion because there 
aren't SMALLINT, TINYINT and so on. I think implicit conversion has the 
disadvantages:
* unnecessary type conversion cost
* (maybe) forget the original type


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

Line 392:fnName_.getFunction().equalsIgnoreCase("trunc") ||
This is required to determine result type.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I856da9f817b948de3c72af60a0742b128398b4cf
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5511: Add process start time to debug web page

[Gabor Kaszab (Code Review)]

Gabor Kaszab has uploaded a new patch set (#6).

Change subject: IMPALA-5511: Add process start time to debug web page
..

IMPALA-5511: Add process start time to debug web page

Read the start date and time of the impalad, catalogd and statestored processes
for the Debug Web UI. Uses the stat command on the /proc/ directory and
format the date with the date command to local time format.

Change-Id: I05ae2f80835b1b0e4bc3b38731778ba0871338a4
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/catalog/catalogd-main.cc
M be/src/runtime/exec-env.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M be/src/util/proc-info-test.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
15 files changed, 177 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I05ae2f80835b1b0e4bc3b38731778ba0871338a4
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..


Patch Set 33: Code-Review+2

Fixed a bug in the PHJ debug string where it didn't handle a closed build 
partition correctly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
Gerrit-PatchSet: 33
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

[Tim Armstrong (Code Review)]

Hello Dan Hecht,

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

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

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

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..

IMPALA-4674: Part 2: port backend exec to BufferPool

Always create global BufferPool at startup using 80% of memory and
limit reservations to 80% of query memory (same as BufferedBlockMgr).
The query's initial reservation is computed in the planner, claimed
centrally (managed by the InitialReservations class) and distributed
to query operators from there.

min_spillable_buffer_size and default_spillable_buffer_size query
options control the buffer size that the planner selects for
spilling operators.

Port ExecNodes to use BufferPool:
  * Each ExecNode has to claim its reservation during Open()
  * Port Sorter to use BufferPool.
  * Switch from BufferedTupleStream to BufferedTupleStreamV2
  * Port HashTable to use BufferPool via a Suballocator.

This also makes PAGG memory consumption more efficient (avoid wasting buffers)
and improve the spilling algorithm:
* Allow preaggs to execute with 0 reservation - if streams and hash tables
  cannot be allocated, it will pass through rows.
* Halve the buffer requirement for spilling aggs - avoid allocating
  buffers for aggregated and unaggregated streams simultaneously.
* Rebuild spilled partitions instead of repartitioning (IMPALA-2708)

TODO in follow-up patches:
* Rename BufferedTupleStreamV2 to BufferedTupleStream
* Implement max_row_size query option.

Testing:
* Updated tests to reflect new memory requirements

Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/analytic-eval-node.cc
M be/src/exec/analytic-eval-node.h
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hash-table-test.cc
M be/src/exec/hash-table.cc
M be/src/exec/hash-table.h
M be/src/exec/hash-table.inline.h
M be/src/exec/nested-loop-join-builder.cc
M be/src/exec/partitioned-aggregation-node-ir.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-ir.cc
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-ir.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/exec/partitioned-hash-join-node.inline.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/CMakeLists.txt
D be/src/runtime/buffered-block-mgr-test.cc
D be/src/runtime/buffered-block-mgr.cc
D be/src/runtime/buffered-block-mgr.h
D be/src/runtime/buffered-tuple-stream-test.cc
D be/src/runtime/buffered-tuple-stream.cc
D be/src/runtime/buffered-tuple-stream.h
D be/src/runtime/buffered-tuple-stream.inline.h
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.h
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/initial-reservations.cc
A be/src/runtime/initial-reservations.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M be/src/runtime/test-env.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/service/client-request-state.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.h
M be/src/util/static-asserts.cc
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/DataStreamSink.java
M fe/src/main/java/org/apache/impala/planner/EmptySetNode.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M 

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6898/1/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:

Line 84:   public static KuduClient getKuduClient(String kuduMasters) {
> how does this deal with invalidated clients? is there some way to detect th
The change has now gone in on the Kudu side to automatically refresh 
invalidated clients (KUDU-2013).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created (FE)

[Thomas Tauber-Marshall (Code Review)]

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

Change subject: IMPALA-5167: Reduce the number of Kudu clients created (FE)
..

IMPALA-5167: Reduce the number of Kudu clients created (FE)

Creating Kudu clients is very expensive as each will fetch
metadata from the Kudu master, so we should minimize the
number of Kudu clients that get created.

This patch stores a map from Kudu master addressed to Kudu
clients in KuduUtil to be used across the FE and catalog.
Another patch has already addressed the BE.

This relies on two changes on the Kudu side: one that clears
non-covered range entries from the client's cache on table
open (d07ecd6ded01201c912d2e336611a6a941f48d98), and one
that automatically refreshes auth tokens when they expire
(603c1578c78c0377ffafdd9c427ebfd8a206bda3).

Testing:
- Ran a stress test on a 10 node cluster: scan of a small
  Kudu table, 1000 concurrent queries, load on the Kudu
  master was reduced signficantly, from ~50% cpu to ~5%.
  (with the BE changes included)
- Ran the Kudu e2e tests.
- Manually ran a test with concurrent INSERTs and
  'ALTER TABLE ADD PARTITION' (which is affected by the
  Kudu side change mentiond above) and verified
  correctness.

Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
---
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
4 files changed, 42 insertions(+), 21 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 

[Impala-ASF-CR] Bump Kudu version to f144d57

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded a new change for review.

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

Change subject: Bump Kudu version to f144d57
..

Bump Kudu version to f144d57

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


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iea7d94f9807fade7dca781814d795265e254b20e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

[anujphadke (Code Review)]

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 171 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-5529: Add additional function signatures for TRUNC()

[Kim Jin Chul (Code Review)]

Kim Jin Chul has uploaded a new change for review.

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

Change subject: IMPALA-5529: Add additional function signatures for TRUNC()
..

IMPALA-5529: Add additional function signatures for TRUNC()

The following signatures to be added:
+--+--+-+---+
| return type  | signature| binary type | is persistent 
|
+--+--+-+---+
| BIGINT   | trunc(BIGINT)| BUILTIN | true  
|
| DECIMAL(*,*) | trunc(DECIMAL(*,*))  | BUILTIN | true  
|
| DECIMAL(*,*) | trunc(DECIMAL(*,*), BIGINT)  | BUILTIN | true  
|
| DECIMAL(*,*) | trunc(DECIMAL(*,*), INT) | BUILTIN | true  
|
| DECIMAL(*,*) | trunc(DECIMAL(*,*), SMALLINT)| BUILTIN | true  
|
| DECIMAL(*,*) | trunc(DECIMAL(*,*), TINYINT) | BUILTIN | true  
|
| BIGINT   | trunc(DOUBLE)| BUILTIN | true  
|
| INT  | trunc(INT)   | BUILTIN | true  
|
| SMALLINT | trunc(SMALLINT)  | BUILTIN | true  
|
| TIMESTAMP| trunc(TIMESTAMP, STRING) | BUILTIN | true  
|
| TINYINT  | trunc(TINYINT)   | BUILTIN | true  
|
+--+--+-+---+

Change-Id: I856da9f817b948de3c72af60a0742b128398b4cf
---
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
2 files changed, 11 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I856da9f817b948de3c72af60a0742b128398b4cf
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 

[Impala-ASF-CR] IMPALA-5659: Begin standardizing treatment of thirdparty libraries

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5659: Begin standardizing treatment of thirdparty 
libraries
..


Patch Set 5:

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/867/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7a6bc5610aaf2450f91348d94cfb984c6a4b78d
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: Tim Armstrong 
Gerrit-HasComments: No