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

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,

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

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

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

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

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

NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.

The original commit message is below:

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: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M CMakeLists.txt
M LICENSE.txt
M be/src/common/kudu_version.cc
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M bin/rat_exclude_files.txt
10 files changed, 150 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

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

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..


Patch Set 17:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2804/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 17
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 12 Jul 2018 04:11:07 +
Gerrit-HasComments: No


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

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,

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

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

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

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

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

NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.

The original commit message is below:

Import FindKRPC.cmake from Apache Kudu.

Add some files to protoc-gen-krpc link to allow it to find symbols now
defined within Impala (without linking all of Impala's libraries).

Change-Id: I5693288db90f2e9673b8c88ca4378c3790cba957
Reviewed-on: http://gerrit.cloudera.org:8080/5719
Reviewed-by: Henry Robinson 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/CMakeLists.txt
M be/src/rpc/impala-service-pool.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/runtime/krpc-data-stream-mgr.cc
4 files changed, 11 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5693288db90f2e9673b8c88ca4378c3790cba957
Gerrit-Change-Number: 10760
Gerrit-PatchSet: 13
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-7211: Fix the between predicate for decimals

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

Change subject: IMPALA-7211: Fix the between predicate for decimals
..

IMPALA-7211: Fix the between predicate for decimals

Before this patch, some queries would fail where the inputs to the
between predicate were decimal types that are not compatible with each
other. We would needlessly cast them to the same type in the FE. This
was not necessary because we are able to handle decimals of different
types in the backend already for this predicate. This patch should even
result in a slight performance improvement.

Testing:
- Added some tests to AnalyzeExprsTest.

Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Reviewed-on: http://gerrit.cloudera.org:8080/10898
Reviewed-by: Vuk Ercegovac 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
3 files changed, 62 insertions(+), 63 deletions(-)

Approvals:
  Vuk Ercegovac: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 10898
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7211: Fix the between predicate for decimals

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

Change subject: IMPALA-7211: Fix the between predicate for decimals
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 10898
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 12 Jul 2018 04:04:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7201. Support DDL with LocalCatalog enabled

2018-07-11 Thread Todd Lipcon (Code Review)
Hello Tianyi Wang, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-7201. Support DDL with LocalCatalog enabled
..

IMPALA-7201. Support DDL with LocalCatalog enabled

This fixes a couple issues with DDL commands when LocalCatalog is
enabled:

- updateCatalogCache() gets called after any DDL. Instead of throwing an
  exception, we can just no-op this by returning some fake result.

- In order to support 'drop database' we need to properly implement the
  various function-related calls such that they don't throw exceptions.
  This changes them to be stubbed out as having no functions.

- Fixes for 'alter view' and 'drop view' so that the underlying target
  table gets loaded by the catalogd before attempting the operation.
  Without this, in the LocalCatalog case, the catalogd would only have
  an IncompleteTable and these operations would fail with "unexpected
  table type" errors.

With this patch I was able to run 'run-tests.py -k views' and 3/4
passed. The one that failed depends on HBase tables, not yet
implemented.

Change-Id: Ic39c97a5f5ad145e03b96d1a470dc2dfa6ec71a5
---
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
3 files changed, 38 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic39c97a5f5ad145e03b96d1a470dc2dfa6ec71a5
Gerrit-Change-Number: 10806
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-2422: Fix escaping in the LIKE clause

2018-07-11 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10910 )

Change subject: IMPALA-2422: Fix escaping in the LIKE clause
..

IMPALA-2422: Fix escaping in the LIKE clause

There are two stages to processing a like clause. First, we determine if
it is possible to convert the expression to a simpler function, such as
StartsWith() or EndsWith(). If not, then we use a Regex libarary to
compute the result.

There was a problem in the logic that determines if it is possible to
use a simpler function. It did not take into account escape characters.
For example, "abc\%" was incorrectly converted to StartsWith("abc\").

There was another problem. We always unescaped strings in the frontend.
The RE2 regex function also unescapes the regex before proceeding. So
regexes were unescaped twice, which caused some ambiguity. For example,
"abc\%" and "abc\\%" are unescaped in the frontend and the same pattern,
"abc\%" is sent to the backend. The backend could not decide if this
pattern is an exact or prefix match. To fix this problem, we avoid
unescaping regex pattens in the frontend.

Testing:
 -Added expr tests.

Change-Id: I553412318525820a36d2f401aa7e93958d22f70e
---
M be/src/exprs/expr-test.cc
M be/src/exprs/like-predicate.cc
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
5 files changed, 68 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I553412318525820a36d2f401aa7e93958d22f70e
Gerrit-Change-Number: 10910
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Reviewed-on: http://gerrit.cloudera.org:8080/10922
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 22 insertions(+), 48 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 12 Jul 2018 02:41:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables

2018-07-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10798 )

Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on 
HDFS tables
..


Patch Set 4:

Appears that some conflict developed in trunk, will rebase this series again :(


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 10798
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 12 Jul 2018 02:37:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7140 (part 6): fetch column stats for LocalTable

2018-07-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10797 )

Change subject: IMPALA-7140 (part 6): fetch column stats for LocalTable
..

IMPALA-7140 (part 6): fetch column stats for LocalTable

This adds fetching of column statistics for LocalTable. Currently, all
column stats are fetched when the table is loaded, even for simple
statements like 'DESCRIBE' where they aren't necessary. This is because
I couldn't find a convenient spot during analysis at which time the set
of necessary columns are known. I left a TODO for this potential
improvement.

With this change I can see that 'SHOW COLUMN STATS' shows the expected
results for functional.alltypes. A new simple unit test verifies this.

Planner tests still don't pass due to some NullPointerExceptions related
to loading functions from the builtins DB -- most of the tests seem to
rely on simple built-ins like COUNT and CAST.

Change-Id: Ib6403c2bedf4ee29c5e6f90e947382cb44f46e0c
Reviewed-on: http://gerrit.cloudera.org:8080/10797
Tested-by: Impala Public Jenkins 
Reviewed-by: Todd Lipcon 
---
M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
7 files changed, 114 insertions(+), 20 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib6403c2bedf4ee29c5e6f90e947382cb44f46e0c
Gerrit-Change-Number: 10797
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7006: Add KRPC folders from kudu@334ecafd

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho,

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

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

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

Change subject: IMPALA-7006: Add KRPC folders from kudu@334ecafd
..

IMPALA-7006: Add KRPC folders from kudu@334ecafd

cp -a ~/checkout/kudu/src/kudu/{rpc,util,security} be/src/kudu/

Change-Id: I232db2b4ccf5df9aca87b21dea31bfb2735d1ab7
---
A be/src/kudu/rpc/CMakeLists.txt
A be/src/kudu/rpc/acceptor_pool.cc
A be/src/kudu/rpc/acceptor_pool.h
A be/src/kudu/rpc/blocking_ops.cc
A be/src/kudu/rpc/blocking_ops.h
A be/src/kudu/rpc/client_negotiation.cc
A be/src/kudu/rpc/client_negotiation.h
A be/src/kudu/rpc/connection.cc
A be/src/kudu/rpc/connection.h
A be/src/kudu/rpc/connection_id.cc
A be/src/kudu/rpc/connection_id.h
A be/src/kudu/rpc/constants.cc
A be/src/kudu/rpc/constants.h
A be/src/kudu/rpc/exactly_once_rpc-test.cc
A be/src/kudu/rpc/inbound_call.cc
A be/src/kudu/rpc/inbound_call.h
A be/src/kudu/rpc/messenger.cc
A be/src/kudu/rpc/messenger.h
A be/src/kudu/rpc/mt-rpc-test.cc
A be/src/kudu/rpc/negotiation-test.cc
A be/src/kudu/rpc/negotiation.cc
A be/src/kudu/rpc/negotiation.h
A be/src/kudu/rpc/outbound_call.cc
A be/src/kudu/rpc/outbound_call.h
A be/src/kudu/rpc/periodic-test.cc
A be/src/kudu/rpc/periodic.cc
A be/src/kudu/rpc/periodic.h
A be/src/kudu/rpc/protoc-gen-krpc.cc
A be/src/kudu/rpc/proxy.cc
A be/src/kudu/rpc/proxy.h
A be/src/kudu/rpc/reactor-test.cc
A be/src/kudu/rpc/reactor.cc
A be/src/kudu/rpc/reactor.h
A be/src/kudu/rpc/remote_method.cc
A be/src/kudu/rpc/remote_method.h
A be/src/kudu/rpc/remote_user.cc
A be/src/kudu/rpc/remote_user.h
A be/src/kudu/rpc/request_tracker-test.cc
A be/src/kudu/rpc/request_tracker.cc
A be/src/kudu/rpc/request_tracker.h
A be/src/kudu/rpc/response_callback.h
A be/src/kudu/rpc/result_tracker.cc
A be/src/kudu/rpc/result_tracker.h
A be/src/kudu/rpc/retriable_rpc.h
A be/src/kudu/rpc/rpc-bench.cc
A be/src/kudu/rpc/rpc-test-base.h
A be/src/kudu/rpc/rpc-test.cc
A be/src/kudu/rpc/rpc.cc
A be/src/kudu/rpc/rpc.h
A be/src/kudu/rpc/rpc_context.cc
A be/src/kudu/rpc/rpc_context.h
A be/src/kudu/rpc/rpc_controller.cc
A be/src/kudu/rpc/rpc_controller.h
A be/src/kudu/rpc/rpc_header.proto
A be/src/kudu/rpc/rpc_introspection.proto
A be/src/kudu/rpc/rpc_service.h
A be/src/kudu/rpc/rpc_sidecar.cc
A be/src/kudu/rpc/rpc_sidecar.h
A be/src/kudu/rpc/rpc_stub-test.cc
A be/src/kudu/rpc/rpcz_store.cc
A be/src/kudu/rpc/rpcz_store.h
A be/src/kudu/rpc/rtest.proto
A be/src/kudu/rpc/rtest_diff_package.proto
A be/src/kudu/rpc/sasl_common.cc
A be/src/kudu/rpc/sasl_common.h
A be/src/kudu/rpc/sasl_helper.cc
A be/src/kudu/rpc/sasl_helper.h
A be/src/kudu/rpc/serialization.cc
A be/src/kudu/rpc/serialization.h
A be/src/kudu/rpc/server_negotiation.cc
A be/src/kudu/rpc/server_negotiation.h
A be/src/kudu/rpc/service_if.cc
A be/src/kudu/rpc/service_if.h
A be/src/kudu/rpc/service_pool.cc
A be/src/kudu/rpc/service_pool.h
A be/src/kudu/rpc/service_queue-test.cc
A be/src/kudu/rpc/service_queue.cc
A be/src/kudu/rpc/service_queue.h
A be/src/kudu/rpc/transfer.cc
A be/src/kudu/rpc/transfer.h
A be/src/kudu/rpc/user_credentials.cc
A be/src/kudu/rpc/user_credentials.h
A be/src/kudu/security/CMakeLists.txt
A be/src/kudu/security/ca/cert_management-test.cc
A be/src/kudu/security/ca/cert_management.cc
A be/src/kudu/security/ca/cert_management.h
A be/src/kudu/security/cert-test.cc
A be/src/kudu/security/cert.cc
A be/src/kudu/security/cert.h
A be/src/kudu/security/crypto-test.cc
A be/src/kudu/security/crypto.cc
A be/src/kudu/security/crypto.h
A be/src/kudu/security/init.cc
A be/src/kudu/security/init.h
A be/src/kudu/security/kerberos_util.cc
A be/src/kudu/security/kerberos_util.h
A be/src/kudu/security/krb5_realm_override.cc
A be/src/kudu/security/openssl_util.cc
A be/src/kudu/security/openssl_util.h
A be/src/kudu/security/openssl_util_bio.h
A be/src/kudu/security/security-test-util.cc
A be/src/kudu/security/security-test-util.h
A be/src/kudu/security/security_flags.cc
A be/src/kudu/security/security_flags.h
A be/src/kudu/security/simple_acl.cc
A be/src/kudu/security/simple_acl.h
A be/src/kudu/security/test/mini_kdc-test.cc
A be/src/kudu/security/test/mini_kdc.cc
A be/src/kudu/security/test/mini_kdc.h
A be/src/kudu/security/test/test_certs.cc
A be/src/kudu/security/test/test_certs.h
A be/src/kudu/security/test/test_pass.cc
A be/src/kudu/security/test/test_pass.h
A be/src/kudu/security/tls_context.cc
A be/src/kudu/security/tls_context.h
A be/src/kudu/security/tls_handshake-test.cc
A be/src/kudu/security/tls_handshake.cc
A be/src/kudu/security/tls_handshake.h
A be/src/kudu/security/tls_socket-test.cc
A be/src/kudu/security/tls_socket.cc
A be/src/kudu/security/tls_socket.h
A be/src/kudu/security/token-test.cc
A be/src/kudu/security/token.proto
A be/src/kudu/security/token_signer.cc
A be/src/kudu/security/token_signer.h
A be/src/kudu/security/token_signing_key.cc
A be/src/kudu/security/token_signing_key.h
A 

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

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,

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

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

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

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

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

NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.

The original commit message is below:

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: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M CMakeLists.txt
M LICENSE.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M bin/rat_exclude_files.txt
9 files changed, 143 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-7212: Deprecate --use krpc flag and remove old DataStream services

2018-07-11 Thread Michael Ho (Code Review)
Hello Sailesh Mukil,

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

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

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

Change subject: IMPALA-7212: Deprecate --use_krpc flag and remove old 
DataStream services
..

IMPALA-7212: Deprecate --use_krpc flag and remove old DataStream services

This change deprecates the flag --use_krpc which allows users
to fall back to using Thrift based implementation of DataStream
services. This flag was originally added during development of
IMPALA-2567. It has served its purpose.

As we port more ImpalaInternalServices to use KRPC, it's becoming
increasingly burdensome to maintain parallel implementation of the
RPC handlers. Therefore, going forward, KRPC is always enabled.
This change removes the Thrift based implemenation of DataStreamServices
and also simplifies some of the tests which were skipped when KRPC
is disabled.

Testing done: core debug build.

Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3
---
M be/src/common/global-flags.cc
M be/src/exec/data-sink.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exchange-node.h
M be/src/rpc/authentication.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/thrift-server-test.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/backend-client.h
D be/src/runtime/data-stream-mgr-base.h
D be/src/runtime/data-stream-mgr.h
D be/src/runtime/data-stream-recvr-base.h
D be/src/runtime/data-stream-recvr.cc
D be/src/runtime/data-stream-recvr.h
D be/src/runtime/data-stream-sender.cc
D be/src/runtime/data-stream-sender.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-recvr.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/scheduling/scheduler.cc
M be/src/service/data-stream-service.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-internal-service.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M common/thrift/ImpalaInternalService.thrift
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
D tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_krpc_metrics.py
M tests/custom_cluster/test_rpc_exception.py
M tests/query_test/test_codegen.py
M tests/webserver/test_web_pages.py
44 files changed, 87 insertions(+), 2,142 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3
Gerrit-Change-Number: 10835
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class

2018-07-11 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10813 )

Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..


Patch Set 6:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h@152
PS6, Line 152:   /// exited the INITALIZED state.
update comment to mention that it blocks till a terminal state has been reached 
(since it call waitForFinishInternal())


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h@156
PS6, Line 156: Same as WaitForPrepareInternal
nit: avoid reference to private methods. how about switching the description 
and adding to it in the private method instead, like "Helper method for 
WaitForPrepare(), does the same but returns a FInstanceStatus instead


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h@281
PS6, Line 281: CANCELLED
when is the cancelled state set?


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.h@289
PS6, Line 289: ERROR
if cancelled, will the status be Status::CANCELLED? if yes that is also counted 
as an error/non-ok state and BackendExecState will be set to ERROR


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.cc@233
PS6, Line 233: TUniqueId failed_instance = finst_status.finst_id_;
unused variable. maybe just pass a status object to this method if the 
finst_id_ is not used.

Also, I noticed that the finst_id_ is stored during call to 
ErrorDuringPrepare() and ErrorDuringExecute() but dosent seem to be used 
anywhere. we can probably get rid of the struct. or am I missing something?


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.cc@241
PS6, Line 241: if (!status.ok()) {
 : // Error while executing - go to ERROR state.
 : query_status_ = finst_status;
 : backend_exec_state_ = BackendExecState::ERROR;
 :   }
do we have a cancellation path?


http://gerrit.cloudera.org:8080/#/c/10813/6/be/src/runtime/query-state.cc@379
PS6, Line 379:   instances_finished_barrier_->Wait();
unique_lock l(status_lock_)?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:45:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7140 (part 8): support views in LocalCatalog

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

Change subject: IMPALA-7140 (part 8): support views in LocalCatalog
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3516b9ceff6dce12ded68d93afde09728627e08
Gerrit-Change-Number: 10805
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:32:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

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

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..


Patch Set 15: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 15
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:19:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-1760: Implement shutdown command

2018-07-11 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10744 )

Change subject: IMPALA-1760: Implement shutdown command
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:

http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py@94
PS11, Line 94: r'quiesce period left: ([0-9ms]*), deadline left: ([0-9ms]*), ' +
 :   r'fragment instances: ([0-9]*), queries registered: 
([0-9]*)'
> This appears in a banner on the front page of the web UI - is that what you
Yes. That's good. Thanks!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0
Gerrit-Change-Number: 10744
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:13:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Pranay Singh (Code Review)
Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10847 )

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..


Patch Set 5:

> Can you also add test case to look for the SIGTERM induced
 > 'shutdown log' in the log file? Since that's the whole point of
 > adding the handler, I think it would be good to ensure we don't
 > lose the log message.

Added the code to check for log message before and after.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:08:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

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

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..

IMPALA-7129: Fixing path to UBSAN suppressions.

This pins the suppression file path at build time.

It turns out that calling getenv("IMPALA_HOME") was returning null at
the time the undefined behavior was being initialized. I suspect this is
an initialization ordering issue. To step around the issue, I'm simply
using a preprocessor macro to get at the suppression file path.

I was able to reproduce the error by running the catalogd binary
outside of $IMPALA_HOME. Before this change, that would fail reliably.

Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Reviewed-on: http://gerrit.cloudera.org:8080/10895
Reviewed-by: Jim Apple 
Tested-by: Impala Public Jenkins 
---
M be/CMakeLists.txt
M be/src/common/init.cc
2 files changed, 8 insertions(+), 5 deletions(-)

Approvals:
  Jim Apple: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 7
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

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

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:08:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Pranay Singh (Code Review)
Hello Lars Volker, Zoram Thanga,

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

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

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

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..

IMPALA-6271: Impala daemon should log a message when it's being shut down

Currently Impalad does not log any message when SIGTERM is sent to
impalad to terminate or to do a graceful shut down. This change logs
a message when SIGTERM is received by impalad/catalogd/statestored.
This logging will assist in debugging the issues seen in the field
where impalad was not gracefully shut down (some other signal
was generated that led to impalad/catalogd/statestored crash).

Testing:
---
a) Used kill to send signals to impalad/catalogd/statestored
   `kill -s SIGTERM ` and see the
   log message is being logged in impalad/catalogd/statestored.INFO.
b) Ran test_breakpad.py to check that existing breakpad functionalities
   are not affected.
c) Ran exhaustive tests without failure.
d) Added new test in test_breakpad.py to handle SIGTERM for
   impalad/statestored/catalogd.

Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
---
M be/src/catalog/catalogd-main.cc
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/minidump.cc
M tests/custom_cluster/test_breakpad.py
5 files changed, 77 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-7211: Fix the between predicate for decimals

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

Change subject: IMPALA-7211: Fix the between predicate for decimals
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2802/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 10898
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 12 Jul 2018 00:42:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB

2018-07-11 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10850 )

Change subject: IMPALA-6086: Use of permanent function should require SELECT 
privilege on DB
..


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/10850/3//COMMIT_MSG@10
PS3, Line 10: leat
nit: least


http://gerrit.cloudera.org:8080/#/c/10850/3//COMMIT_MSG@18
PS3, Line 18:
add a "Testing: " section header.


http://gerrit.cloudera.org:8080/#/c/10850/3//COMMIT_MSG@24
PS3, Line 24: private parameterized Jenkins job, exhaustive exploration and
: covering all tests
nit: replace with "Ran exhaustive and covering tests."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 12 Jul 2018 00:40:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7251: Fix QueryMaintenance calls in Aggregators

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10871 )

Change subject: IMPALA-7251: Fix QueryMaintenance calls in Aggregators
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4dac2bb0a15cdd7315ee15608bae409c125c82f5
Gerrit-Change-Number: 10871
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 12 Jul 2018 00:27:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10415 )

Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query
..


Patch Set 9:

Could you take another look, Bikram?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 9
Gerrit-Owner: Mostafa Mokhtar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 12 Jul 2018 00:23:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-1760: Implement shutdown command

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10744 )

Change subject: IMPALA-1760: Implement shutdown command
..


Patch Set 11:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/10744/11//COMMIT_MSG
Commit Message:

PS11:
RE: "I have a question that should we call CheckNotShuttingDown() in 
ImpalaInternalService::ExecQueryFInstances? If somehow the quiesce period is 
too short that coordinators still schedule fragment instances to the shutting 
down node, the queries can be failed fast."

I kind of like the suggestion but I'd be concerned about adding another code 
path to test - for the moment it seems simpler to just have one code path for 
failing queries that run past the quiesce period.


http://gerrit.cloudera.org:8080/#/c/10744/11//COMMIT_MSG@26
PS11, Line 26: e.g. statestore down
> Could you add tests for this? Could you explain more about how the shutdown
I added a basic test that kills the statestore. This property is pretty trivial 
to verify by looking at the code - the shutdown code path doesn't communicate 
with the statestore or refer to the cluster membership or anything like that.


http://gerrit.cloudera.org:8080/#/c/10744/11//COMMIT_MSG@29
PS11, Line 29: * If shutting down, a banner is shown on the root debug page.
> does this get exposed programatically somehow as well? I would think that c
You can get it in JSON form from the root debug page, i.e. 
http://host:port/?json=true, although that might not be the "right" interface. 
I thought about adding more functions to query status, etc but decided not to 
do that in this patch because it felt a little speculative and the patch is 
already large.


http://gerrit.cloudera.org:8080/#/c/10744/11//COMMIT_MSG@32
PS11, Line 32: 1. (if a coordinator) clients are prevented from submitting
 :   queries to this coordinator via some out-of-band mechanism,
 :   e.g. load balancer
> should shutting-down coordinators reject new queries or new sessions after
The current patch starts rejecting new queries and sessions immediately once a 
coordinator is shut down. Clarified here.


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

http://gerrit.cloudera.org:8080/#/c/10744/11/be/src/service/client-request-state.cc@628
PS11, Line 628:   for (int i = 0; i < 3; ++i) {
> What about sleep several seconds before the next retry like this?
Seems like a good idea to consider but I don't want to make the change in this 
patch (we want to keep this consistent with the backend exec RPC). I filed a 
JIRA to track this IMPALA-7283

To that end though, I removed the code duplication with the other place that 
does retry to make it easier to make such changes in the future.


http://gerrit.cloudera.org:8080/#/c/10744/11/be/src/util/default-path-handlers.cc
File be/src/util/default-path-handlers.cc:

http://gerrit.cloudera.org:8080/#/c/10744/11/be/src/util/default-path-handlers.cc@228
PS11, Line 228: bool is_quiescing = impala_server->IsShuttingDown();
> I think for now we should just stick to is_shutting_down since this stateme
I think that the Impala daemon can be quiesceing even after the period has 
elapsed - it's only successfully quiesced once nothing is running on it. So 
that really means that the quiesce period is the minimum quiesce period. 
Updated some comments accordingly.


http://gerrit.cloudera.org:8080/#/c/10744/11/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/10744/11/common/thrift/StatestoreService.thrift@79
PS11, Line 79: it
> nit: typo
Done


http://gerrit.cloudera.org:8080/#/c/10744/11/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
File fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java:

http://gerrit.cloudera.org:8080/#/c/10744/11/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java@95
PS11, Line 95:* Supports optionally specifying the backend and the 
deadline: either shutdown(),
 :* shutdown('host:port'), shutdown(deadline), 
shutdown('host:port', deadline).
> Cool! It'd be better to mention these in the commit message:
Updated the commit message.


http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py
File tests/custom_cluster/test_restart_services.py:

http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py@94
PS11, Line 94: r'quiesce period left: ([0-9ms]*), deadline left: ([0-9ms]*), ' +
 :   r'fragment instances: ([0-9]*), queries registered: 
([0-9]*)'
> I'd be great if these can be shown in the web page dynamically.
This appears in a banner on the front page of the web UI - is that what you 
were asking for?


http://gerrit.cloudera.org:8080/#/c/10744/11/tests/custom_cluster/test_restart_services.py@212
PS11, Line 

[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:25:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2801/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:25:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles

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

Change subject: IMPALA-7095: clean up scan node profiles
..


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:19:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles

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

Change subject: IMPALA-7095: clean up scan node profiles
..

IMPALA-7095: clean up scan node profiles

Add counters to scan node implementations where they make sense but were
missing (e.g. row batch queue counters for multithread Kudu scans) and
remove them where they don't make sense (e.g. scanner thread counters
for non-multithreaded scans).

Refactors the multithreaded Kudu and HDFS scans to share logic via
composition (single inheritance doesn't work for this case),
which enables the same set of counters to be maintained with shared
code. The row batch queueing and thread tracking is now shared. I looked
at combining the logic around 'status_', 'lock_' and 'done_' between the
two but the details were different enough that it didn't seem worth
abstracting.

Adds a PeakScannerThreadConcurrency counter - this answers a common
question.

Fixes RowsRead for data source scans.

Fix some of the comments to be more accurate/useful.

Testing:
Ran exhaustive tests. Ran various types of scans (HDFS, Kudu, HBase,
Data source) and inspected the profile output manually.

Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Reviewed-on: http://gerrit.cloudera.org:8080/10810
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/data-source-scan-node.cc
M be/src/exec/data-source-scan-node.h
M be/src/exec/hbase-scan-node.cc
M be/src/exec/hbase-scan-node.h
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node-mt.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/hdfs-text-scanner.cc
M be/src/exec/kudu-scan-node-base.h
M be/src/exec/kudu-scan-node-mt.cc
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/exec/scan-node.cc
M be/src/exec/scan-node.h
M be/src/runtime/fragment-instance-state.cc
M be/src/util/blocking-queue.h
M be/src/util/thread.h
20 files changed, 496 insertions(+), 357 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:16:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB

2018-07-11 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10850 )

Change subject: IMPALA-6086: Use of permanent function should require SELECT 
privilege on DB
..


Patch Set 3:

(5 comments)

looks fine, just some minor additional comments.

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@481
PS3, Line 481: LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
> I think that's the right approach in general: Parse the statement, then che
ok. looks like this change is an improvement over the current behavior. pls 
file a jira to make such checks earlier.


http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2070
PS3, Line 2070: //
nit: remove


http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2074
PS3, Line 2074: //
nit: remove


http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2085
PS3, Line 2085: // Enable expression rewrite
move above L2082?


http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2153
PS3, Line 2153: ScalarFunction fn = ScalarFunction.createForTesting(db, fnName,
  : new ArrayList(), Type.INT, "/dummy", 
"dummy.class", null,
  : null, TFunctionBinaryType.NATIVE);
  : authzCatalog_.addFunction(fn);
replace with a call to the more generic function on L2160



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:08:36 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py@85
PS2, Line 85:   return (times['h'] * 60 * 60 + times['m'] * 60 + times['s']) * 
1000 + times['ms']
> Nit: long line.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:07:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread Bikramjeet Vig (Code Review)
Hello Michael Brown, David Knupp,

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

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

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 22 insertions(+), 48 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10847 )

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..


Patch Set 4:

Can you also add test case to look for the SIGTERM induced 'shutdown log' in 
the log file? Since that's the whole point of adding the handler, I think it 
would be good to ensure we don't lose the log message.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:03:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB

2018-07-11 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10850 )

Change subject: IMPALA-6086: Use of permanent function should require SELECT 
privilege on DB
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@481
PS3, Line 481: LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
> supposing the folded fn reveals something interesting, e.g., getSSN("some u
I think that's the right approach in general: Parse the statement, then check 
access, then perform analysis. I think that would be well outside the scope of 
this JIRA, though. Please also note that this code was existing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:59:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

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

Change subject: IMPALA-7239: Disable smaps parsing by default
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:44:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

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

Change subject: IMPALA-7239: Disable smaps parsing by default
..

IMPALA-7239: Disable smaps parsing by default

Accessing smaps has proven to be too expensive to enable on all
operating systems. Let's move the metrics behind a hidden flag
so that they can be enabled for development if needed but
won't affect production workloads.

Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Reviewed-on: http://gerrit.cloudera.org:8080/10884
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/common/init.cc
M be/src/util/memory-metrics.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
4 files changed, 66 insertions(+), 46 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/2/tests/util/parse_util.py@85
PS2, Line 85:   return times['h'] * 60 * 60 * 1000 + times['m'] * 60 * 1000 + 
times['s'] * 1000 + times[
Nit: long line.

Maybe you can factor out the multiple "* 1000"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:43:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7275: Create table authorization error should not show table name

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

Change subject: IMPALA-7275: Create table authorization error should not show 
table name
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453
Gerrit-Change-Number: 10919
Gerrit-PatchSet: 5
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:40:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables

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

Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on 
HDFS tables
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 10798
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:19:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7140 (part 8): support views in LocalCatalog

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

Change subject: IMPALA-7140 (part 8): support views in LocalCatalog
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2800/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3516b9ceff6dce12ded68d93afde09728627e08
Gerrit-Change-Number: 10805
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:09:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7140 (part 6): fetch column stats for LocalTable

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

Change subject: IMPALA-7140 (part 6): fetch column stats for LocalTable
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6403c2bedf4ee29c5e6f90e947382cb44f46e0c
Gerrit-Change-Number: 10797
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:09:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

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

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..


Patch Set 15:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2799/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 15
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:05:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10769 )

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..


Patch Set 15: Code-Review+2

Rebased and addressed the final comment, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 15
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10763 )

Change subject: IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork
..


Patch Set 11:

That carried +2 was from Sailesh, obviously. :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3c6e933c454e7adca69ef03e7d5c0c84b656895
Gerrit-Change-Number: 10763
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:05:09 +
Gerrit-HasComments: No


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

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10758 )

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


Patch Set 10: Code-Review+2

Rebased and addressed the final comment. Carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] KUDU-2305: Limit sidecars to INT MAX and fortify socket code

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10765 )

Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code
..


Patch Set 11: Code-Review+2

Rebased, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id23e518995f2bf2f6bf6b49d5f413f3eaa4e79d1
Gerrit-Change-Number: 10765
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:36 +
Gerrit-HasComments: No


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

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10760 )

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


Patch Set 11: Code-Review+2

Rebased, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5693288db90f2e9673b8c88ca4378c3790cba957
Gerrit-Change-Number: 10760
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10763 )

Change subject: IMPALA-5129: Use Kudu's Kinit code to avoid expensive fork
..


Patch Set 11: Code-Review+2

Rebased, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3c6e933c454e7adca69ef03e7d5c0c84b656895
Gerrit-Change-Number: 10763
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
..


Patch Set 10: Code-Review+2

Rebased, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Add KRPC folders from kudu@334ecafd

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10757 )

Change subject: IMPALA-7006: Add KRPC folders from kudu@334ecafd
..


Patch Set 7: Code-Review+2

Rebased, carrying Michael's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I232db2b4ccf5df9aca87b21dea31bfb2735d1ab7
Gerrit-Change-Number: 10757
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:03:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7212: Deprecate --use krpc flag and remove old DataStream services

2018-07-11 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10835 )

Change subject: IMPALA-7212: Deprecate --use_krpc flag and remove old 
DataStream services
..


Patch Set 2:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@23
PS1, Line 23: #include "common/status.h"
> I think I've found all the dead code in this patch, but if you want a refer
Thanks for checking.


http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@128
PS1, Line 128: class DataStreamTest : public testing::Test {
 :  protected:
 :   DataStreamTest() : next_val_(0) {
 : // Stop tests that rely on mismatched sender / receiver 
pairs timing out from failing.
 :
> No longer needed.
Done


http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@134
PS1, Line 134:   ~DataStreamTest() { runtime_state_->ReleaseResources(); }
 :
 :   virtual void SetUp() {
 :
> No longer needed.
Done


http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@139
PS1, Line 139: erPool(32 * 1024, 1024 * 1024 * 1024, 32 * 1024);
> You can replace this with:
Done


http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py
File tests/custom_cluster/test_rpc_exception.py:

http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py@a73
PS1, Line 73:
> Can't this happen without TransmitData() today?  Don't we also call it on R
ReportExecStatus() can be very timing sensitive as the number of iterations 
it's invoked depends on how long the query runs for.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3
Gerrit-Change-Number: 10835
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:02:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7006: Remove KRPC folders

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10756 )

Change subject: IMPALA-7006: Remove KRPC folders
..


Patch Set 5: Code-Review+2

Rebased, carrying Michael's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic677484c27ed18b105da0a6b0901df4eb9f248e6
Gerrit-Change-Number: 10756
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:02:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho, Sailesh Mukil, Joe McDonnell, Dan Hecht,

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

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

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

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..

IMPALA-7006: Pick parts of recent Kudu gutil changes

- Include some ASAN macros from gutil (Kudu commit c8724c61)
- Pick parts of KUDU-2427 (Kudu commit b7cf3b2e)
- Rename constants (Kudu commit e719b5ef)

These changes will be subsumed by a proper rebase of GUTIL.

Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
---
M be/src/gutil/macros.h
M be/src/gutil/port.h
M be/src/gutil/strings/substitute.cc
M be/src/gutil/strings/substitute.h
M be/src/gutil/sysinfo.cc
M be/src/util/error-util.h
6 files changed, 100 insertions(+), 40 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 15
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


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

2018-07-11 Thread Lars Volker (Code Review)
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,

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

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

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

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

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

NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.

The original commit message is below:

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: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
7 files changed, 79 insertions(+), 31 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 


[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB

2018-07-11 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10850 )

Change subject: IMPALA-6086: Use of permanent function should require SELECT 
privilege on DB
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:

http://gerrit.cloudera.org:8080/#/c/10850/3/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@481
PS3, Line 481: LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
supposing the folded fn reveals something interesting, e.g., getSSN("some user 
name") ... this approach evaluates it and outputs it to the log. while I don't 
think we output this rewritten query in an error (or possibly elsewhere 
downstream), have you looked at avoiding the evaluation of fn in the first 
place if access is not permitted? the approach here seems prone to currently 
leak and can get worse depending on future changes.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:55:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

2018-07-11 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10895 )

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:45:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

2018-07-11 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10895 )

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..


Patch Set 6:

The typo broke my testing. This time I tested it by removing the suppressions 
file and getting the expected file not found error. If it's in the right place, 
no error. Turns out you need to do an extra layering of quoting. This makes 
sense since CMake is generating makefiles which are doing shell-style 
evaluation.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:42:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

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

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2798/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:41:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Pranay Singh (Code Review)
Hello Lars Volker, Zoram Thanga,

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

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

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

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..

IMPALA-6271: Impala daemon should log a message when it's being shut down

Currently Impalad does not log any message when SIGTERM is sent to
impalad to terminate or to do a graceful shut down. This change logs
a message when SIGTERM is received by impalad/catalogd/statestored.
This logging will assist in debugging the issues seen in the field
where impalad was not gracefully shut down (some other signal
was generated that led to impalad/catalogd/statestored crash).

Testing:
---
a) Used kill to send signals to impalad/catalogd/statestored
   `kill -s SIGTERM ` and see the
   log message is being logged in impalad/catalogd/statestored.INFO.
b) Ran test_breakpad.py to check that existing breakpad functionalities
   are not affected.
c) Ran exhaustive tests without failure.
d) Added new test in test_breakpad.py to handle SIGTERM for
   impalad/statestored/catalogd.

Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
---
M be/src/catalog/catalogd-main.cc
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/minidump.cc
M tests/custom_cluster/test_breakpad.py
5 files changed, 67 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-7129: Fixing path to UBSAN suppressions.

2018-07-11 Thread Philip Zeyliger (Code Review)
Hello Fredy Wijaya, Jim Apple, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7129: Fixing path to UBSAN suppressions.
..

IMPALA-7129: Fixing path to UBSAN suppressions.

This pins the suppression file path at build time.

It turns out that calling getenv("IMPALA_HOME") was returning null at
the time the undefined behavior was being initialized. I suspect this is
an initialization ordering issue. To step around the issue, I'm simply
using a preprocessor macro to get at the suppression file path.

I was able to reproduce the error by running the catalogd binary
outside of $IMPALA_HOME. Before this change, that would fail reliably.

Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
---
M be/CMakeLists.txt
M be/src/common/init.cc
2 files changed, 8 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5295c2d1bddbea585a761b85a51eadecf10d191d
Gerrit-Change-Number: 10895
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Pranay Singh (Code Review)
Hello Lars Volker, Zoram Thanga,

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

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

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

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..

IMPALA-6271: Impala daemon should log a message when it's being shut down

Currently Impalad does not log any message when SIGTERM is sent to
impalad to terminate or to do a graceful shut down. This change logs
a message when SIGTERM is received by impalad/catalogd/statestored.
This logging will assist in debugging the issues seen in the field
where impalad was not gracefully shut down (some other signal
was generated that led to impalad/catalogd/statestored crash).

Testing:
---
a) Used kill to send signals to impalad/catalogd/statestored
   `kill -s SIGTERM ` and see the
   log message is being logged in impalad/catalogd/statestored.INFO.
b) Ran test_breakpad.py to check that existing breakpad functionalities
   are not affected.
c) Ran exhaustive tests without failure.
d) Added new test in test_breakpad.py to handle SIGTERM for
   impalad/statestored/catalogd.

Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
---
M be/src/catalog/catalogd-main.cc
M be/src/service/impalad-main.cc
M be/src/statestore/statestored-main.cc
M be/src/util/minidump.cc
M tests/custom_cluster/test_breakpad.py
5 files changed, 67 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 3
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down

2018-07-11 Thread Pranay Singh (Code Review)
Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10847 )

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
..


Patch Set 2:

(9 comments)

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

http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@9
PS2, Line 9:   Currently Impalad does not log any message when SIGTERM is sent 
to impalad
> nit: flush left
Done


http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@11
PS2, Line 11: recieved
> nit: typo
Done


http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@12
PS2, Line 12: shutdown
> nit: two words
Done


http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@14
PS2, Line 14:   Testing:
> This change should have an automatic test, possibly in custom-cluster-tests
Done


http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc
File be/src/service/impalad-main.cc:

http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@62
PS2, Line 62: static void HandlerTerm(int signum)
> nit: formatting ({ on same line). You can run clang-format with the config
Done


http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@64
PS2, Line 64:LOG(INFO) << "SIGTERM encountered invoking default handler 
system going down" << endl;
> I would change the wording to "Caught SIGTERM. Daemon going down."
Done


http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108
PS2, Line 108: old_action
> Do we currently set a handler for SIGTERM elsewhere? If not, we should remo
No we don't handle the SIGTERM elsewhere I have removed the old_action now I 
exit the process after handling SIGTERM


http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108
PS2, Line 108: old_action
> Please add a comment explaining why SIGTERM is specifically handled (i.e.,
Done


http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc
File be/src/util/minidump.cc:

http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc@101
PS2, Line 101: recieved
> nit: typo
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:33:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

2018-07-11 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10918 )

Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at 
analysis
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:15:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7006: Pick parts of recent Kudu gutil changes

2018-07-11 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10769 )

Change subject: IMPALA-7006: Pick parts of recent Kudu gutil changes
..


Patch Set 14:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10769/14//COMMIT_MSG@11
PS14, Line 11: Rename constants
> Can you please also specify the commit (e719b5ef) ?
Will do in final push.

  - Rename constants (Kudu commit e719b5ef)


http://gerrit.cloudera.org:8080/#/c/10769/14/be/src/gutil/strings/substitute.cc
File be/src/gutil/strings/substitute.cc:

http://gerrit.cloudera.org:8080/#/c/10769/14/be/src/gutil/strings/substitute.cc@15
PS14, Line 15:
> Is the change in gutil/strings/escaping.cc missing ?(https://github.com/apa
I only picked those parts of that commit that had an effect on Impala's code, 
notably renaming NoArg to kNoArg. Parts that didn't affect Impala's inclusion 
of the Kudu code were left for a later, proper rebase of gutil.

Not picking this commit at all would have meant to undo parts of it in Kudu's 
code.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2dc8c70425e3ac030427ebeb1ec18a44d14d5cb
Gerrit-Change-Number: 10769
Gerrit-PatchSet: 14
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:14:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10918 )

Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at 
analysis
..

IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

In order to do COMPUTE STATS, Impala performs several SELECT queries.
However, in the COMPUTE STATS analysis phase, we only register for the
ALTER privilege. Although the SELECT privilege is eventually registered
to the target table by the SELECT child queries, it is better to
register the SELECT privilege in the COMPUTE STATS analysis phase and
fail early if the privilege requirements are not met instead of failing
later in the SELECT child queries due to insufficient privileges.

Testing:
- Updated the authorization tests
- Ran all FE tests

Change-Id: Icead43e689404a286859344e309427eb6c68646a
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
7 files changed, 91 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 1:

(2 comments)

Thanks David!

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@76
PS1, Line 76: r'(?P[0-9]+(\.[0-9]+)?)(?P\D+)'
> There's something weird about your pattern. If I print out the result of ap
Done


http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@83
PS1, Line 83: if (match[2] == 'h'):
> Also, rather than the if/elif construction, you could maybe do this:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:03:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread Bikramjeet Vig (Code Review)
Hello Michael Brown, David Knupp,

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

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

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

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 23 insertions(+), 48 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 


[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10918 )

Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at 
analysis
..

IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

In order to do COMPUTE STATS, Impala performs several SELECT queries.
However, in the COMPUTE STATS analysis phase, we only register for the
ALTER privilege. Although the SELECT privilege is eventually registered
to the target table by the SELECT child queries, it is better to register
the SELECT privilege in the COMPUTE STATS analysis phase and fail early
if the privilege requirements are not met instead of failing later in the
SELECT child queries due to insufficient privileges.

Testing:
- Updated the authorization tests
- Ran all FE tests

Change-Id: Icead43e689404a286859344e309427eb6c68646a
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
7 files changed, 91 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10918 )

Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at 
analysis
..


Patch Set 2:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/10918/1//COMMIT_MSG@15
PS1, Line 15: SELEC
> nit: SELECT
Done


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

http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2384
PS1, Line 2384: Always registers privilege request(s) for the table at the 
given privilege level(s),
  :* regardless of the state of the table (i.e. whether it 
exists, is loaded, etc.).
  :* If addAccessEvent is true adds access event(s) for 
successfully loaded tables.
> nit: Update comment to reflect multiple privileges and access events.
Done


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1185
PS1, Line 1185: nodb.notbl"), onDatabase("nodb", allExcept(
  : TPrivilegeLevel.ALL, TPrivilegeLevel.ALTER,
> missing TPrivilegeLevel.ALTER - alterError("nodb.notbl")
Done


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1193
PS1, Line 1193: functional.notbl"), onDatabase("functional", allExcept(
  : TPrivilegeLevel.ALL, TPrivilegeLevel.ALTER,
> same as above - alterError("functional.notbl")
Done


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1217
PS1, Line 1217: nodb.notbl"), onDatabase("nodb", allExcept(
  : TPrivilegeLevel.ALL, TP
> same as above - alterError("nodb.notbl")
Done


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1225
PS1, Line 1225: ("functional.notbl"), onDatabase("functional", allExcept(
  : TPrivilegeLevel.ALL, TP
> same as above - alterError("functional.notbl")
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 20:55:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class

2018-07-11 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10813 )

Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..


Patch Set 6:

(15 comments)

http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-exec-mgr.cc@128
PS4, Line 128:
> This seems to fit into IMPALA-4063 better. As the code stands in this patch
I removed this and moved the code inside StartFInstances(). I'll bring this 
back with IMPALA-4063


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:

http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@159
PS4, Line 159:  until all
> IMPALA-4063
Removed this.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@278
PS4, Line 278: /// CANCELLED: This query received a CancelQueryFInstances() 
RPC or was directed by
> No implication on whether all fragment instances have finished cancelling t
Yes, added a line that explains that.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@292
PS4, Line 292: ackendExecS
> Did you mean IMPALA-4063 ? Does it mean this comment needs to be removed. M
Yes I meant IMPALA-4063. Removed this.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@350
PS4, Line 350: /// created in St
> std::unique_ptr
Done


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@355
PS4, Line 355: /// whether they
> std::unique_ptr
Done


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@a350
PS4, Line 350:
 :
> Was this a bug ?! Will we hang in the for loop at line 363 below if we fail
This technically is a race, but we never hit it because none of the fragment 
instances try to access fis_map_. I changed it back to the original way.

The reason I had this change in the first place was because one of my 
intermediate patches hit that error, but I don't need this change now.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@141
PS4, Line 141: instances_prepared_barrier_
> As discussed offline, the behavior during "preparation" phase of the query
I changed this based on our offline discussion.

The gist is that I removed the FIS::prepared_promise_ since it was unnecessary 
and introduced a 'status_lock_' and a 'prepared_status_'. Each FIS will try to 
update 'prepared_status_' on its own when it hits an error.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@225
PS4, Line 225: DCHECK(false
> DCHECK(false) ?! Does this warrant a targeted BE test ?
Oops, fixed. I don't think we'll be able to write a meaningful BE test for the 
state machine, because we'll be forcing it into cases we never expect the code 
to hit, just to see an error message.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@230
PS4, Line 230: s QueryState::UpdateBackendExecState(
> This function seems like a no-op in this patch. Can this be added in the ne
Yes, I removed it.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@278
PS4, Line 278:   ReportExecStatusAux(done, status, fis, true);
 : }
 :
 : void QueryState::ReportExecStatusAux(bool done, const Status& 
status,
 : FragmentInstanceState* fis, bool instances_started) {
 :   // if we're reporting an error, we're done
 :   DCHECK(status.ok() || done
> Will this actually be duplicating the error message which we usually log al
The first operator to hit an error will log the error itself. This is a query 
wide acknowledgement of that error in an executor. Subsequent errors hit in the 
same query won't show up here.

We have a pattern of multiple places logging the same error already, and I 
think it's helpful if we have an acknowledgement of the query status of a query 
in an executor.

Anyway, since we discussed and agreed to remove this, I'll do so.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@391
PS4, Line 391: Status status = DescriptorTbl::Create(
> As discussed offline, there is no need to have a different implementation f
Changed based on our offline discussion.


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@434
PS4, Line 434: rence count
> IMPALA-4063 ?
Done


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@502
PS4, Line 502: initiate cancellation if nobody
> Please see comments in query-exec-mgr.cc
Done


http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@542
PS4, Line 542:
 :
> Not sure what you think 

[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class

2018-07-11 Thread Sailesh Mukil (Code Review)
Hello Michael Ho, Joe McDonnell, Dan Hecht,

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

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

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

Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..

IMPALA-7163: Implement a state machine for the QueryState class

This patch adds a state machine for the QueryState class. The motivation
behind this patch is to make the query lifecycle from the point of
view of an executor much easier to reason about and this patch is key
for a follow on patch for IMPALA-2990 where the status reporting will
be per-query rather than per-fragment-instance. Currently, the state
machine provides no other purpose, and it will mostly be used for
IMPALA-2990.

We introduce 5 possible states for the QueryState which include 3
terminal states (FINISHED, CANCELLED and ERROR) and 2 non-terminal
states (INITIALIZED, PREPARED). The transition from one state to the
next is always handled by a single thread which is also the QueryState
thread. This thread will additionally bear the purpose of sending
periodic updates after IMPALA-4063, which is the primary reason behind
having only this thread modify the state of the query.

2 counting barriers are introduced which are used to indicate how many
fragment instances have finished Preparing and Executing. We
use CountingBarriers to allow fragment instance threads to communicate
their completion of their respective states or errors with the QueryState
thread. Due to this design, it's possible for the fragment instances to
have collectively moved on to future states while the query state thread
lags behind in realizing that. However, that's not a concern for us
since we only care about showing a unified view of what's happening on
this executor to the coordinator (post IMPALA-2990).

The status reporting protocol has not been changed and remains exactly
as it was.

Testing: Ran 'core' and 'exhaustive' tests. TODO: Run stress tests.

Future related work:
1) IMPALA-2990: Make status reporting per-query.
2) Try to logically align the FIS states with the QueryState states.
3) Consider mirroring the QueryState state machine to CoordinatorBackendState

Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
---
M be/src/runtime/coordinator.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
5 files changed, 290 insertions(+), 61 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis

2018-07-11 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10918 )

Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at 
analysis
..


Patch Set 1:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/10918/1//COMMIT_MSG@15
PS1, Line 15: SELCT
nit: SELECT


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

http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2384
PS1, Line 2384: Always registers a privilege request for the table at the given 
privilege level,
  :* regardless of the state of the table (i.e. whether it 
exists, is loaded, etc.).
  :* If addAccessEvent is true adds an access event for 
successfully loaded tables.
nit: Update comment to reflect multiple privileges and access events.


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1185
PS1, Line 1185: default.nodb"), onDatabase("nodb", allExcept(
  : TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)
missing TPrivilegeLevel.ALTER - alterError("nodb.notbl")


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1193
PS1, Line 1193: default.functional"), onDatabase("functional", allExcept(
  : TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)
same as above - alterError("functional.notbl")


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1217
PS1, Line 1217: default.nodb"), onDatabase("nodb", allExcept(
  : TPrivilegeLevel.ALL)));
same as above - alterError("nodb.notbl")


http://gerrit.cloudera.org:8080/#/c/10918/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1225
PS1, Line 1225: ("default.functional"), onDatabase("functional", allExcept(
  : TPrivilegeLevel.ALL)));
same as above - alterError("functional.notbl")



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a
Gerrit-Change-Number: 10918
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 20:32:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7201. Support DDL with LocalCatalog enabled

2018-07-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10806 )

Change subject: IMPALA-7201. Support DDL with LocalCatalog enabled
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10806/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/10806/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1397
PS3, Line 1397: catalog_.getOrLoadTable(params.getTable_name().db_name,
this seems to have changed the behavior of 'DROP TABLE IF EXISTS 
some_non_existent_db.foo' to throw an exception instead of returning "Database 
does not exist" below. Will work on that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic39c97a5f5ad145e03b96d1a470dc2dfa6ec71a5
Gerrit-Change-Number: 10806
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 20:27:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10810 )

Change subject: IMPALA-7095: clean up scan node profiles
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10810/8/be/src/exec/scan-node.h
File be/src/exec/scan-node.h:

http://gerrit.cloudera.org:8080/#/c/10810/8/be/src/exec/scan-node.h@227
PS8, Line 227: Not thread-safe.
> Is it expected that the caller is supposed to hold certain locks before cal
It depends on external synchronisation (lock_ in the various scan node 
implementations)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 11 Jul 2018 20:07:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10810 )

Change subject: IMPALA-7095: clean up scan node profiles
..


Patch Set 10: Code-Review+2

Carry


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 11 Jul 2018 20:07:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

2018-07-11 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10923 )

Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and 
DESCRIBE DATABASE
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
Gerrit-Change-Number: 10923
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:58:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@83
PS1, Line 83: if (match[2] == 'h'):
Also, rather than the if/elif construction, you could maybe do this:

  pattern = r'(?P[0-9]+\.?[0-9]*?)(?P\D+)'
  matches = list(re.finditer(pattern, duration))   # You need list() here if 
you plan to assert
  assert matches, 'Failed to parse duration string %s' % duration

  times = {'h': 0, 'm': 0, 's': 0, 'ms': 0}
  for match in matches:
parsed = match.groupdict()
times[parsed['units']] = float(parsed['value'])

  return times['h'] * 60 * 60 * 1000 + times['m'] * 60 * 1000 + times['s'] * 
1000 + times['ms']



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:40:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10923 )

Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and 
DESCRIBE DATABASE
..

IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

In DESCRIBE DATABASE, having VIEW_METADATA privilege allows seeing the
metadata information on the target database. Similarly, other SQL show
commands require VIEW_METADATA privilege on the target database/table.
In the prior code, DESCRIBE requires SELECT privilege on the target table
and is inconsistent with the rest of other SQL metadata commands. The
patch fixes the inconsistency by requiring DESCRIBE to use VIEW_METADATA
privilege.

Testing:
- Updated authorization tests
- Ran all FE tests

Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
2 files changed, 23 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
Gerrit-Change-Number: 10923
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10923 )

Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and 
DESCRIBE DATABASE
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1059
PS1, Line 1059: SEL
> I think this should stay SELECT since that is the only privilege we allow o
Done


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1086
PS1, Line 1086: SEL
> Same as above
Done


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1134
PS1, Line 1134: SEL
> same as above
Done


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1145
PS1, Line 1145: SEL
> same as above
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
Gerrit-Change-Number: 10923
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:40:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles

2018-07-11 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10810 )

Change subject: IMPALA-7095: clean up scan node profiles
..


Patch Set 8: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node-base.h
File be/src/exec/kudu-scan-node-base.h:

http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node-base.h@41
PS7, Line 41:
:   virtual Status Prepare(RuntimeState* state) override;
:   virtual Status Open(RuntimeState* state) override;
:   virtual Status GetNext(RuntimeState* state, RowBatch* 
row_batch, bool* eos)
:   override = 0;
:  protected:
:   virtual void DebugString(int indentation_level, 
std::stringstream* out) const override;
> It's not really necessary now that the clang precommit builds detect droppe
It seems that we have them in some places (hdfs-scan-node-base.h) in this patch 
but not others. Just a bit inconsistent.


http://gerrit.cloudera.org:8080/#/c/10810/8/be/src/exec/scan-node.h
File be/src/exec/scan-node.h:

http://gerrit.cloudera.org:8080/#/c/10810/8/be/src/exec/scan-node.h@227
PS8, Line 227: Not thread-safe.
Is it expected that the caller is supposed to hold certain locks before calling 
this function or is it called from a particular thread context only ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:33:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

2018-07-11 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10923 )

Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and 
DESCRIBE DATABASE
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1059
PS1, Line 1059: ALL
I think this should stay SELECT since that is the only privilege we allow on 
columns.  If we're looking to future proof, then it should just be the metadata 
privileges.


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1086
PS1, Line 1086: ALL
Same as above


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1134
PS1, Line 1134: ALL
same as above


http://gerrit.cloudera.org:8080/#/c/10923/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1145
PS1, Line 1145: ALL
same as above



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
Gerrit-Change-Number: 10923
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:25:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

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

Change subject: IMPALA-7239: Disable smaps parsing by default
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2796/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:23:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7275: Create table authorization error should not show table name

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

Change subject: IMPALA-7275: Create table authorization error should not show 
table name
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2795/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453
Gerrit-Change-Number: 10919
Gerrit-PatchSet: 5
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:22:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

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

Change subject: IMPALA-7239: Disable smaps parsing by default
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:23:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7275: Create table authorization error should not show table name

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10919 )

Change subject: IMPALA-7275: Create table authorization error should not show 
table name
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453
Gerrit-Change-Number: 10919
Gerrit-PatchSet: 5
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:21:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7275: Create table authorization error should not show table name

2018-07-11 Thread Adam Holley (Code Review)
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10919 )

Change subject: IMPALA-7275: Create table authorization error should not show 
table name
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10919/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/10919/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@124
PS4, Line 124: Since we don't get back the modified
 : // authorizable list, we need to check again and modify 
the error
> nit: I think this comment is no longer relevant.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453
Gerrit-Change-Number: 10919
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:20:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10922 )

Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/10922/1/tests/util/parse_util.py@76
PS1, Line 76: r'(?P[0-9]+(\.[0-9]+)?)(?P\D+)'
There's something weird about your pattern. If I print out the result of 
applying re.findall() to your sample duration string, each match winds up being 
a tuple with 3 elements:

  [('1', '', 'h'), ('2', '', 'h'), ('3', '', 'm'), ('4', '', 's'), ('5.6', 
'.6', 'ms'), ('4.5', '.5', 'us'), ('7.8', '.8', 'ns')]

...when you *probably* want each tuple to have 2 elements -- just the value and 
the units, like this:

  [('1', 'h'), ('2', 'h'), ('3', 'm'), ('4', 's'), ('5.6', 'ms'), ('4.5', 
'us'), ('7.8', 'ns')]

I think this pattern will give you that (but I'm not a regex expert):

  r'(?P[0-9]+\.?[0-9]*?)(?P\D+)'

Furthermore, you're using the semantics for named groups (i.e., ?P and 
?P), but then resorting to referencing by index numbers. You could try 
something like this:

  pattern = r'(?P[0-9]+\.?[0-9]*?)(?P\D+)'
  matches = re.finditer(pattern, duration)

  for match in matches:
parsed = match.groupdict()
if parsed['units'] == 'h':
  hours = float(parsed['value'])
etc...

...which is a little more readable than [0] and [2].



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:18:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10908 )

Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements
..


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java:

http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@91
PS2, Line 91: stmt.getClass().getSimpleName() + " statements");
:   }
> Personally, I think the exception make sense when some external event cause
Thanks for the explanation!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
Gerrit-Change-Number: 10908
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Comment-Date: Wed, 11 Jul 2018 19:02:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements

2018-07-11 Thread Pooja Nilangekar (Code Review)
Pooja Nilangekar has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/10908 )

Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements
..

IMPALA-7209: Disallow self referencing in ALTER VIEW statements

Previously, ALTER VIEW queries did not carry out reference checks
in the analysis phase. This allowed the DDL operation to succeed
but subsequent queries to the view threw StackOverflowError
because the catalog was unable to resolve the reference. With this
change, the AlterViewStmt checks for direct and in-direct self
references before altering a view.

Testing: Added tests to AnalyzeDDLTest to verify it.
Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
---
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
2 files changed, 58 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
Gerrit-Change-Number: 10908
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Pooja Nilangekar 


[Impala-ASF-CR] IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables

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

Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on 
HDFS tables
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2794/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 10798
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 11 Jul 2018 18:55:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

2018-07-11 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10884 )

Change subject: IMPALA-7239: Disable smaps parsing by default
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 11 Jul 2018 18:53:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

2018-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10884 )

Change subject: IMPALA-7239: Disable smaps parsing by default
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10884/2/be/src/util/process-state-info.cc@178
PS2, Line 178: string ProcessStateInfo::DebugString() const {
> I dunno where this is used, but would it make sense to not include the bits
It's used on the debug page. Fixed it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 11 Jul 2018 18:48:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7239: Disable smaps parsing by default

2018-07-11 Thread Tim Armstrong (Code Review)
Hello Todd Lipcon,

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

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

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

Change subject: IMPALA-7239: Disable smaps parsing by default
..

IMPALA-7239: Disable smaps parsing by default

Accessing smaps has proven to be too expensive to enable on all
operating systems. Let's move the metrics behind a hidden flag
so that they can be enabled for development if needed but
won't affect production workloads.

Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
---
M be/src/common/init.cc
M be/src/util/memory-metrics.cc
M be/src/util/process-state-info.cc
M be/src/util/process-state-info.h
4 files changed, 66 insertions(+), 46 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b
Gerrit-Change-Number: 10884
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10908 )

Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java:

http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@65
PS2, Line 65: TableRef tblRef = new TableRef(tableName_.toPath(), null);
: tblRef = analyzer.resolveTableRef(tblRef);
nit: can be simplified to TableRef tblRef = analyzer.resolveTableRef(new 
TableRef(tableName_.toPath(), null));


http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@79
PS2, Line 79: Self reference
nit: Self-reference


http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@91
PS2, Line 91: throw new AnalysisException("Subqueries not supported for " +
: stmt.getClass().getSimpleName() + " statements");
I'm a bit torn whether we should throw an AnalysisException or have a 
Preconditions check since this condition should never happen given that the 
parser doesn't support this syntax. Let me know what you think?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
Gerrit-Change-Number: 10908
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-Comment-Date: Wed, 11 Jul 2018 18:36:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10923


Change subject: IMPALA-7059: Inconsistent privilege between DESCRIBE and 
DESCRIBE DATABASE
..

IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

In DESCRIBE DATABASE, having VIEW_METADATA privilege allows seeing the
metadata information on the target database. Similarly, other SQL show
commands require VIEW_METADATA privilege on the target database/table.
In the prior code, DESCRIBE requires SELECT privilege on the target table
and is inconsistent with the rest of other SQL metadata commands. The
patch fixes the inconsistency by requiring DESCRIBE to use VIEW_METADATA
privilege.

Testing:
- Updated authorization tests
- Ran all FE tests

Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
2 files changed, 27 insertions(+), 36 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
Gerrit-Change-Number: 10923
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-7279: Fix flakiness in test rows availability

2018-07-11 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10922


Change subject: IMPALA-7279: Fix flakiness in test_rows_availability
..

IMPALA-7279: Fix flakiness in test_rows_availability

This patch fixes a flaky time string parsing method in
test_rows_availability that fails on strings with microsecond precision.

Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
---
M tests/query_test/test_hash_join_timer.py
M tests/query_test/test_rows_availability.py
M tests/util/parse_util.py
3 files changed, 28 insertions(+), 48 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If7634869823d8cc4059048dd5d3c3a984744f3be
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 


[Impala-ASF-CR] IMPALA-7275: Create table authorization error should not show table name

2018-07-11 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10919 )

Change subject: IMPALA-7275: Create table authorization error should not show 
table name
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10919/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/10919/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@124
PS4, Line 124: Since we don't get back the modified
 : // authorizable list, we need to check again and modify 
the error
nit: I think this comment is no longer relevant.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453
Gerrit-Change-Number: 10919
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Wed, 11 Jul 2018 17:07:11 +
Gerrit-HasComments: Yes


  1   2   >