date:20180205

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9186 )

Change subject: IMPALA-6456: Add flags to configure rpc_negotiation_timeout_ms 
and negotiation thread count in KRPC
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9186/2/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9186/2/be/src/rpc/rpc-mgr-test.cc@537
PS2, Line 537: auto save_rpc_negotiation_timeout_ms = 
FLAGS_rpc_negotiation_timeout_ms;
Should this use something like ScopedFlagSetter() in case the ASSERT below 
fires ?


http://gerrit.cloudera.org:8080/#/c/9186/2/be/src/rpc/rpc-mgr-test.cc@541
PS2, Line 541: tls_krpc_address
Is TLS always enabled ? Seem a bit misleading to have the "tls_" prefix here 
and below.


http://gerrit.cloudera.org:8080/#/c/9186/2/be/src/rpc/rpc-mgr-test.cc@551
PS2, Line 551: secondary_rpc_mgr.Shutdown();
Just wondering if it'll be a problem if ASSERT() above gets triggered. Will we 
exit without calling Shutdown() here ?


http://gerrit.cloudera.org:8080/#/c/9186/1/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9186/1/be/src/rpc/rpc-mgr.cc@84
PS1, Line 84:   
bld.set_rpc_negotiation_timeout_ms(FLAGS_rpc_negotiation_timeout_ms);
> The min number of threads is 0 by default. What that means is that there wo
Makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I108d700e7eac04b678e21a3a920aac81ba8eede5
Gerrit-Change-Number: 9186
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 06 Feb 2018 07:42:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..

IMPALA-6473: Fix analytic fn that partitions and orders on same expr

Previously, an analytic function that used the same expr in both the
'partition by' and 'order by' clauses, and where the expr meets the
criteria for being materialized before the sort, would hit an
IllegalStateException due to the expr being inserted into the same
ExprSubstitutionMap twice.

If the values have already been partitioned on the expr, then all of
the values for it in each partition will be the same and also ordering
on the expr doesn't change the results. So, the fix is to simply
exclude the duplicate expr from the 'order by' while still
partitioning on it.

Testing:
- Added a regression test to PlannerTest.

Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Reviewed-on: http://gerrit.cloudera.org:8080/9218
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
3 files changed, 33 insertions(+), 6 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 06 Feb 2018 03:25:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5037: Default PARQUET ARRAY RESOLUTION=THREE LEVEL

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9210 )

Change subject: IMPALA-5037: Default PARQUET_ARRAY_RESOLUTION=THREE_LEVEL
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py
File tests/query_test/test_nested_types.py:

http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py@134
PS1, Line 134: ARRAY_RESOUTION_POLICIES
> Typo: RESOLUTION
Done


http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py@208
PS1, Line 208: self.client.execute("set parquet_array_resolution=%s" % 
arr_res)
> Will this setting stick around when the client is recycled? Should we be re
Good point. Switched to self.execute_query() which internally calls uses 
self.client.set_configuration().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8f7e9010c4354d667305d9df7b78862efb23fe1
Gerrit-Change-Number: 9210
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 02:18:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5037: Default PARQUET ARRAY RESOLUTION=THREE LEVEL

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5037: Default PARQUET_ARRAY_RESOLUTION=THREE_LEVEL
..

IMPALA-5037: Default PARQUET_ARRAY_RESOLUTION=THREE_LEVEL

Changes the default value for the PARQUET_ARRAY_RESOLUTION
query option to conform to the Parquet standard.
Before: TWO_LEVEL_THEN_THREE_LEVEL
After:  THREE_LEVEL

For more information see:
* IMPALA-4725
* https://github.com/apache/parquet-format/blob/master/LogicalTypes.md

Testing:
- expands and cleans up the existing tests for more coverage
  over the different resolution policies
- private core/hdfs run passed

Cherry-picks: not for 2.x.

Change-Id: Ib8f7e9010c4354d667305d9df7b78862efb23fe1
---
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/query_test/test_nested_types.py
3 files changed, 155 insertions(+), 193 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib8f7e9010c4354d667305d9df7b78862efb23fe1
Gerrit-Change-Number: 9210
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6075: Add Impala daemon metric for catalog version.

2018-02-05 Thread Pranay Singh (Code Review)

Hello Dimitris Tsirogiannis, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6075: Add Impala daemon metric for catalog version.
..

IMPALA-6075: Add Impala daemon metric for catalog version.

This patch adds new metrics for current version of catalog, current catalog 
topic version
and catalog service id which are currently used by impala daemon.

Testing:
Verified manually that the new metrics for catalog version, catalog topic 
version,
catalog service id are displayed and they corresponds to the latest version in
catalogd.INFO log file.

Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
M common/thrift/metrics.json
5 files changed, 62 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Gerrit-Change-Number: 8949
Gerrit-PatchSet: 9
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6075: Add Impala daemon metric for catalog version.

2018-02-05 Thread Pranay Singh (Code Review)

Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8949 )

Change subject: IMPALA-6075: Add Impala daemon metric for catalog version.
..


Patch Set 8:

> Looks like the API for Metric changed
 >
 > -fsanitize=thread -DTHREAD_SANITIZER -fverbose-asm -D_GNU_SOURCE
 > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
 > -fPIC   -fPIC -MD -MT 
 > be/src/service/CMakeFiles/Service.dir/impala-server.cc.o
 > -MF be/src/service/CMakeFiles/Service.dir/impala-server.cc.o.d -o
 > be/src/service/CMakeFiles/Service.dir/impala-server.cc.o -c
 > be/src/service/impala-server.cc
 > 01:58:23 ] be/src/service/impala-server.cc:1325:36: error: no
 > member named 'set_value' in 
 > 'impala::AtomicMetric';
 > did you mean 'SetValue'?
 > 01:58:23 ]   ImpaladMetrics::CATALOG_VERSION->set_value(catalog_version);
 > 01:58:23 ]^
 > 01:58:23 ]SetValue
 > 01:58:23 ] be/src/util/metrics.h:233:8: note: 'SetValue' declared
 > here
 > 01:58:23 ]   void SetValue(const int64_t& value) {
 > value_.Store(value); }
 > 01:58:23 ]^
 > 01:58:23 ] be/src/service/impala-server.cc:1326:42: error: no
 > member named 'set_value' in 
 > 'impala::AtomicMetric';
 > did you mean 'SetValue'?
 > 01:58:23 ]   
 > ImpaladMetrics::CATALOG_TOPIC_VERSION->set_value(catalog_topic_version);
 > 01:58:23 ]  ^
 > 01:58:23 ]  SetValue
 > 01:58:23 ] be/src/util/metrics.h:233:8: note: 'SetValue' declared
 > here
 > 01:58:23 ]   void SetValue(const int64_t& value) {
 > value_.Store(value); }
 > 01:58:23 ]^
 > 01:58:23 ] be/src/service/impala-server.cc:1327:39: error: no
 > member named 'set_value' in 'impala::LockedMetric impala::TMetricKind::type::PROPERTY>'; did you mean 'SetValue'?
 > 01:58:23 ]   
 > ImpaladMetrics::CATALOG_SERVICE_ID->set_value(PrintId(catalog_service_id));
 > 01:58:23 ]   ^
 > 01:58:23 ]   SetValue
 > 01:58:23 ] be/src/util/metrics.h:197:8: note: 'SetValue' declared
 > here
 > 01:58:23 ]   void SetValue(const T& value) {
 > 01:58:23 ]^
 > 01:58:23 ] 3 errors generated.
 > 01:58:23 ] ninja: build stopped: subcommand failed.
 > 01:58:23 ] Error in /home/ubuntu/Impala/bin/make_impala.sh at line
 > 178: ${MAKE_CMD} ${MAKE_ARGS}

Yes the API for METRIC changed that caused it to fail I have made the correction


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Gerrit-Change-Number: 8949
Gerrit-PatchSet: 8
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:42:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5037: Default PARQUET ARRAY RESOLUTION=THREE LEVEL

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9210 )

Change subject: IMPALA-5037: Default PARQUET_ARRAY_RESOLUTION=THREE_LEVEL
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py
File tests/query_test/test_nested_types.py:

http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py@134
PS1, Line 134: ARRAY_RESOUTION_POLICIES
Typo: RESOLUTION


http://gerrit.cloudera.org:8080/#/c/9210/1/tests/query_test/test_nested_types.py@208
PS1, Line 208: self.client.execute("set parquet_array_resolution=%s" % 
arr_res)
Will this setting stick around when the client is recycled? Should we be 
resetting it?

Should we also be using client.set_configuration() instead of executing the 
server-side statement?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8f7e9010c4354d667305d9df7b78862efb23fe1
Gerrit-Change-Number: 9210
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 01:25:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..


Patch Set 14: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 00:32:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..


Patch Set 11:

(1 comment)

Carry +1

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@651
PS11, Line 651:
  :   vector entry_versions = 
topic->Put(update.topic_entries);
  :   if (!subscriber->AddTransientEntries(
  :   update.topic_name, update.topic_entries, 
entry_versions)) {
  : // Subscriber was unregistered - clean up the transient 
entries.
  : for (int i = 0; i < update.topic_entries.size(); ++i) {
  :   topic->DeleteIfVersionsMatch(entry_versions[i], 
update.topic_entries[i].key);
  : }
  :   }
> Nevermind. I neglected the topic concurrency when thinking about it.
Added a comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 00:32:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h@356
PS11, Line 356: GetSubscribedTopics
> works for me
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Feb 2018 00:31:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Hello Tianyi Wang, Dimitris Tsirogiannis, Alex Behm, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..

IMPALA-6437: separate AC/scheduler from catalog topic updates

This adds a set of "prioritized" statestore topics that are small but
are important to deliver in a timely manner. These are delivered more
frequently by a separate thread pool to reduce the window for stale
admission control and scheduling information.

The contract between statestore and subscriber is changed so that the
statestore can send concurrent Update() RPCs for disjoint sets of
topics. This required changes to the subscriber implementation, which
assumed that only one Update RPC would arrive at a time.

It also changes the locking in the statestore so that the prioritized
update threads don't get stuck behind the catalog threads holding
'topic_lock_'. Specifically, it uses a reader-writer lock to protect
modification of the set of topics and a reader-writer lock per topic to
allow the topic data to be read by multiple threads concurrently.

Added metrics to monitor the per-topic update interval.

Testing:
Ran core tests.

Inspected metrics on Impala daemons, saw that membership and request
queue processing times had more samples recorded than the catalog
topic, reflecting the increased frequency.

Ran under thread sanitizer, made sure no data races were reported in
Statestore or StatestoreSubscriber.

Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M common/thrift/metrics.json
M tests/custom_cluster/test_admission_controller.py
M tests/statestore/test_statestore.py
M www/statestore_subscribers.tmpl
M www/statestore_topics.tmpl
15 files changed, 845 insertions(+), 485 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-02-05 Thread Taras Bobrovytsky (Code Review)

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 10:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/6023/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/6023/10//COMMIT_MSG@10
PS10, Line 10: width_bucket(expr decimal, min_val decimal, max_val decimal, 
num_buckets int)
nit: long line


http://gerrit.cloudera.org:8080/#/c/6023/10//COMMIT_MSG@13
PS10, Line 13: is divided into num_buckets buckets having identical sizes. This 
function
nit: long line


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@514
PS9, Line 514:   dist_from_min = expr.template Subtract(input_scale, 
min_range, input_scale,
> Right, this overflow won't happen without overflowing the subtraction on l
If we never expect this to happen, make this a DCHECK and add a comment.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@557
PS9, Line 557:   return BigIntVal::null();
> Moved it above.
I think it's bad practice to have checks "for safety". It makes the code slower 
and harder to reason about for people who are reading the code. If something is 
impossible, we should not be checking for it. Add a DCHECK instead.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@438
PS10, Line 438: buckets
nit: should be num_buckets instead of buckets


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@444
PS10, Line 444: result
nit: "results"


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@455
PS10, Line 455: a
nit: "an overflow"


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@482
PS10, Line 482:   if (min_range >= max_range) {
UNLIKELY


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@483
PS10, Line 483: ctx->SetError("Lower bound cannot be greater than or equal 
to the upper bound");
Add an end to end test for each error message.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@487
PS10, Line 487:   if (expr < min_range) return 0;
UNLIKELY


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@489
PS10, Line 489:   if (expr >= max_range) {
UNLIKELY


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@504
PS10, Line 504:   input_scale, input_precision, input_scale, false, 
);
UNLIKELY(overflow)


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@505
PS10, Line 505: f(
I think this check needs to happen above the line 487. Imagine if this 
overflows, but expr < min_range, then we will return zero. If we increase expr 
to be equal to min_range, this function will start returning null.

Add a test case for this.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@507
PS10, Line 507: error_msg << "Overflow while evaluating the difference 
between min_range: " <<
Add an end to end test for this error message (and others).


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@518
PS10, Line 518: error_msg << "Overflow while evaluating the difference 
between expr: " <<
Add an end to end test for this error message.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@536
PS10, Line 536:   if (abs(range_size.value()) * 
DecimalUtil::GetScaleMultiplier(input_scale) <
I am not convinced this check is correct. In which case will this check be 
false?


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@538
PS10, Line 538: needs_int256 = true;
Is it possible to tell statically that needs_int256 is false? For example, by 
looking at the precision and scale of min_range and max_range.

We want to order the checks from fastest to slowest. First, do the static 
check. If static check did not rule out int256, do the leading zero check. If 
the leading zero check did not rule out the need for int256, do this check. If 
All the checks did not rule out int256, then set needs_int256 to true.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@566
PS10, Line 566: int128_t y = 
DecimalUtil::MultiplyByScale(range_size.value(),
We need to DCHECK that it did not overflow here. (It should be easy to do if 
you rebase this patch because I modified this function recently and added an 
optional overflow dcheck)


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions.h
File be/src/exprs/math-functions.h:

[native-toolchain-CR](cdh5.x) Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9220


Change subject: Bump Kudu version to b315d0e
..

Bump Kudu version to b315d0e

Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
---
M buildall.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9220
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall

[native-toolchain-CR](cdh5.x) Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9220 )

Change subject: Bump Kudu version to b315d0e
..


Patch Set 1: Code-Review+2

Clean cherry-pick


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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9220
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:44:43 +
Gerrit-HasComments: No

[native-toolchain-CR](cdh5.x) Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9220 )

Change subject: Bump Kudu version to b315d0e
..


Patch Set 1: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: cdh5.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9220
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:44:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:43:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:16:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9218/1/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
File fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java:

http://gerrit.cloudera.org:8080/#/c/9218/1/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java@342
PS1, Line 342: // If the expr is in the PARTITION BY and already in 
'sortExprs', but also in
> minor suggestion:
Done


http://gerrit.cloudera.org:8080/#/c/9218/1/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test:

http://gerrit.cloudera.org:8080/#/c/9218/1/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test@2086
PS1, Line 2086:
> Since the bug was in the planner, please make this a planner test.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 23:15:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Hello Alex Behm,

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

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

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

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..

IMPALA-6473: Fix analytic fn that partitions and orders on same expr

Previously, an analytic function that used the same expr in both the
'partition by' and 'order by' clauses, and where the expr meets the
criteria for being materialized before the sort, would hit an
IllegalStateException due to the expr being inserted into the same
ExprSubstitutionMap twice.

If the values have already been partitioned on the expr, then all of
the values for it in each partition will be the same and also ordering
on the expr doesn't change the results. So, the fix is to simply
exclude the duplicate expr from the 'order by' while still
partitioning on it.

Testing:
- Added a regression test to PlannerTest.

Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
---
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
3 files changed, 33 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9032 )

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..

IMPALA-6219: Use AES-GCM for spill-to-disk encryption

AES-GCM can be very fast(~10 times faster than CFB+SHA256), but it
requires an instruction that Impala can currently run without (CLMUL).
In order to be fast, we dispatch to GCM mode at run-time based on the
CPU and OpenSSL version.

Testing:
run runtime tmp-file-mgr-test, openssl-util-test, buffer-pool-test
and buffered-tuple-stream-test.
add two cases GcmIntegrity & EncryptoArbitraryLength for
openssl-util-test

Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Reviewed-on: http://gerrit.cloudera.org:8080/9032
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/runtime/tmp-file-mgr.cc
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
M be/src/util/openssl-util-test.cc
M be/src/util/openssl-util.cc
M be/src/util/openssl-util.h
6 files changed, 227 insertions(+), 73 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Gerrit-Change-Number: 9032
Gerrit-PatchSet: 16
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9032 )

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..


Patch Set 15: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Gerrit-Change-Number: 9032
Gerrit-PatchSet: 15
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 05 Feb 2018 22:58:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6314: Add run time scalar subquery check for uncorrelated subqueries

Alex Behm has posted comments on this change. (
http://gerrit.cloudera.org:8080/9005 )

Change subject: IMPALA-6314: Add run time scalar subquery check for
uncorrelated subqueries
..

Patch Set 11:

(24 comments)

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.h
File be/src/exec/cardinality-check-node.h:

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.h@27
PS11, Line 27: /// Node that passes along the row pulled from its child
unchanged.
First sentence should crisply state purpose of this node, e.g.:

Node that returns an error if its child produces more than a single row. If
successful, this node returns a deep copy of its single input row.

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.h@29
PS11, Line 29: /// Note that this node must be a blocking node!
Please give an explanation why, e.g.:

Note that this node must be a blocking node. It would be incorrect to return
rows before the single row constraint has been validated because downstream
exec nodes might produce results and incorrectly return them to the client. If
the child of this node produces more than one row it means the SQL query is
semantically invalid, so no rows must be returned to the client.

http://gerrit.cloudera.org:8080/#/c/9005/9/be/src/exec/cardinality-check-node.h
File be/src/exec/cardinality-check-node.h:

http://gerrit.cloudera.org:8080/#/c/9005/9/be/src/exec/cardinality-check-node.h@29
PS9, Line 29: /// Note that this node must be a blocking node!
> No, I just wanted make it more generic. Done.
Thanks.

For reference, I think the "Speculative Generality" code smell applies here.

https://cwiki.apache.org/confluence/display/IMPALA/Effective+Coding+Practices

http://gerrit.cloudera.org:8080/#/c/9005/9/be/src/exec/cardinality-check-node.h@38
PS9, Line 38: virtual Status Reset(RuntimeState* state) override;
> Thanks for the tips. However, I still couldn't succeed. I used the followin
Nice experiments and tests! Seems really hard to hit this case with a real
non-subquery plan. I think documenting the blocking nature of the node in code
comments should be enough.

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.cc
File be/src/exec/cardinality-check-node.cc:

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.cc@68
PS11, Line 68: DCHECK_LE(rows_collected, 1);
DCHECK that rows_collected is either 0 or 1. The current check accepts negative
values.

http://gerrit.cloudera.org:8080/#/c/9005/11/be/src/exec/cardinality-check-node.cc@85
PS11, Line 85: row_batch_->DeepCopyTo(output_row_batch);
Why do we deep copy the row twice? Once in Open() should be sufficient.

Copying twice could be expensive in subplans.

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

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@175
PS11, Line 175: StmtRewriter.rewriteNonScalarSubqueries(operand,
analyzer);
We should still keep the analysis and rewrite phases distinctly separate as
before. Calling ExprRewriter within analysis breaks the separation and is very
confusing.

The StmtRewriter should treat all Exprs in the same way, so no special code
should be required for individual Expr subclasses.

The rewrite should be performed within the rewrite phase when
StmtRewriter.rewrite() is called.

See AnalaysisContext.analyze() for an overview of the phases analysis and
rewrite phases.

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

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@177
PS11, Line 177: for (Expr expr: children_) {
Move to the rewrite phase.

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

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@529
PS11, Line 529: for (Expr child : getChildren()) {
Move to rewrite phase.

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

http://gerrit.cloudera.org:8080/#/c/9005/11/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@94
PS11, Line 94: // This member contains the original statement provided by the
user.
Not accurate. This contains the post-analysis toSql() before rewrites. It may
still be different from the SQL originally provided by the user (table

[Impala-ASF-CR] IMPALA-6448: Re-enable kerberized testing with KRPC

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9164 )

Change subject: IMPALA-6448: Re-enable kerberized testing with KRPC
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6412978316de90875c98f8fbe51c8d215c227b18
Gerrit-Change-Number: 9164
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 22:49:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6448: Re-enable kerberized testing with KRPC

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9164 )

Change subject: IMPALA-6448: Re-enable kerberized testing with KRPC
..

IMPALA-6448: Re-enable kerberized testing with KRPC

For the patch for IMPALA-5054, we realized that we needed to make
the kudu::rpc::Messenger configurable. A patch was done on the Kudu
side which is tracked by KUDU-2228. As part of that patch, one of
the design decisions taken was to only allow kerberos either on or
off for the entirety of the process life. This means that we cannot
switch kerberos on and off in the same process any more with KRPC.
This behavior can be found in SaslInit() in kudu/rpc/sasl_common.cc
as SaslInit() which is called once per process will hard code some
configuration which cannot be toggled.

This affected our kerberized rpc-mgr-tests. This patch splits out
the kerberized part of rpc-mgr-test into rpc-mgr-kerberized-test.

It also puts the common code between both the files into
rpc-mgr-test-base.h

Change-Id: I6412978316de90875c98f8fbe51c8d215c227b18
Reviewed-on: http://gerrit.cloudera.org:8080/9164
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
M be/src/rpc/CMakeLists.txt
A be/src/rpc/rpc-mgr-kerberized-test.cc
A be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/rpc-mgr-test.cc
4 files changed, 380 insertions(+), 316 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6412978316de90875c98f8fbe51c8d215c227b18
Gerrit-Change-Number: 9164
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9218 )

Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9218/1/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
File fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java:

http://gerrit.cloudera.org:8080/#/c/9218/1/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java@342
PS1, Line 342: // If the expr is in the 'partition by' and already in 
'sortExprs', but also in
minor suggestion:

Exprs that appear in both PARTITION BY and ORDER BY only need to be added to 
the 'sortExprs' once.


http://gerrit.cloudera.org:8080/#/c/9218/1/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test:

http://gerrit.cloudera.org:8080/#/c/9218/1/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test@2086
PS1, Line 2086: # IMPALA-6473: analytic fn where the same expr is in the 
'partition by' and the 'order by'
Since the bug was in the planner, please make this a planner test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
Gerrit-Change-Number: 9218
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Mon, 05 Feb 2018 22:36:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6473: Fix analytic fn that partitions and orders on same expr

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9218


Change subject: IMPALA-6473: Fix analytic fn that partitions and orders on same 
expr
..

IMPALA-6473: Fix analytic fn that partitions and orders on same expr

Previously, an analytic function that used the same expr in both the
'partition by' and 'order by' clauses, and where the expr meets the
criteria for being materialized before the sort, would hit an
IllegalStateException due to the expr being inserted into the same
ExprSubstitutionMap twice.

If the values have already been partitioned on the expr, then all of
the values for it in each partition will be the same and also ordering
on the expr doesn't change the results. So, the fix is to simply
exclude the duplicate expr from the 'order by' while still
partitioning on it.

Testing:
- Added a regression test to analytic-fns.test

Change-Id: Id5f1d5fbc6f69df5850f96afed345ce27668c30b
---
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
2 files changed, 24 insertions(+), 3 deletions(-)



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

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

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

2018-02-05 Thread Tianyi Wang (Code Review)

Tianyi Wang has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/9168 )

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

IMPALA-5690: Upgrade thrift to 0.9.3-p3

Dependency changes:
- BE and python use thrift 0.9.3-p3 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Notable code change:
- Thrift 0.9.3 implements "ostream& operator<<(ostream&, T)" for thrift
  data types. It conflicts with impala-defined functions. This patch
  removes these impala-defined functions and changes the call sites
  accordingly.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Change-Id: I9c303997411237e988ef960157f781776f6fcb60
---
M CMakeLists.txt
M be/src/benchmarks/scheduler-benchmark.cc
M be/src/codegen/llvm-codegen.cc
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-metadata-utils.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/rpc-trace.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-thread.h
M be/src/rpc/thrift-util.cc
M be/src/rpc/thrift-util.h
M be/src/runtime/client-cache.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/data-stream-mgr.cc
M be/src/runtime/data-stream-recvr.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-recvr.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-state.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/scheduler.cc
M be/src/service/child-query.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-internal-service.cc
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-result-set.cc
M be/src/statestore/statestore.cc
M be/src/util/collection-metrics.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M be/src/util/histogram-metric.h
M be/src/util/metrics.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
M be/src/util/webserver.cc
M bin/impala-config.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M fe/pom.xml
M infra/python/deps/compiled-requirements.txt
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/shell/test_shell_commandline.py
62 files changed, 330 insertions(+), 443 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c303997411237e988ef960157f781776f6fcb60
Gerrit-Change-Number: 9168
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tianyi Wang

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

2018-02-05 Thread Tianyi Wang (Code Review)

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..


Patch Set 13: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h@356
PS11, Line 356: GetSubscribedTopics
> What about something like GetTopicsMapForId()? That seems to describe more
works for me


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@651
PS11, Line 651:
  : LOG(INFO) << "Received request for different delta base 
of topic: "
  :   << update.topic_name << " from: " << 
subscriber->id()
  :   << " subscriber from_version: " << 
update.from_version;
  : 
subscriber->SetLastTopicVersionProcessed(topic_it->first, update.from_version);
  :   }
  :
  :   Topic& topic = topic_it->second;
  :   v
> I don't necessarily want to hold 'transient_entry_lock_' for the duration o
Nevermind. I neglected the topic concurrency when thinking about it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Feb 2018 20:28:06 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Quiet down file transfers, rename target platform variable

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9149 )

Change subject: Quiet down file transfers, rename target platform variable
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa4349a8ae0ab5bde2dabf5ff65db0f2db4b9ba9
Gerrit-Change-Number: 9149
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:54:10 +
Gerrit-HasComments: No

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

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9217


Change subject: Bump Kudu version to b315d0e
..

Bump Kudu version to b315d0e

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



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

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

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 5: Code-Review+1

(1 comment)

lgtm, will give others a chance to look

http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:

http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@50
PS4, Line 50: EQ("=", "eq"),
> extdatasources would get a serialized version of operator stuff handed over
nice



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 5
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:45:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Hello Tianyi Wang, Dimitris Tsirogiannis, Alex Behm, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..

IMPALA-6437: separate AC/scheduler from catalog topic updates

This adds a set of "prioritized" statestore topics that are small but
are important to deliver in a timely manner. These are delivered more
frequently by a separate thread pool to reduce the window for stale
admission control and scheduling information.

The contract between statestore and subscriber is changed so that the
statestore can send concurrent Update() RPCs for disjoint sets of
topics. This required changes to the subscriber implementation, which
assumed that only one Update RPC would arrive at a time.

It also changes the locking in the statestore so that the prioritized
update threads don't get stuck behind the catalog threads holding
'topic_lock_'. Specifically, it uses a reader-writer lock to protect
modification of the set of topics and a reader-writer lock per topic to
allow the topic data to be read by multiple threads concurrently.

Added metrics to monitor the per-topic update interval.

Testing:
Ran core tests.

Inspected metrics on Impala daemons, saw that membership and request
queue processing times had more samples recorded than the catalog
topic, reflecting the increased frequency.

Ran under thread sanitizer, made sure no data races were reported in
Statestore or StatestoreSubscriber.

Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M common/thrift/metrics.json
M tests/custom_cluster/test_admission_controller.py
M tests/statestore/test_statestore.py
M www/statestore_subscribers.tmpl
M www/statestore_topics.tmpl
15 files changed, 843 insertions(+), 485 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6437: separate AC/scheduler from catalog topic updates

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9123 )

Change subject: IMPALA-6437: separate AC/scheduler from catalog topic updates
..


Patch Set 11:

(13 comments)

Addressed most of the comments but had follow-on questions for two of them.

http://gerrit.cloudera.org:8080/#/c/9123/6/be/src/statestore/statestore-subscriber.h
File be/src/statestore/statestore-subscriber.h:

http://gerrit.cloudera.org:8080/#/c/9123/6/be/src/statestore/statestore-subscriber.h@193
PS6, Line 193:
> update_lock
Done


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.h@356
PS11, Line 356: GetSubscribedTopics
> This name is confusing. Maybe GetPriorityClass()
What about something like GetTopicsMapForId()? That seems to describe more 
exactly what it's doing.


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:

http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.h@71
PS10, Line 71: the same update
> GatherTopicUpdates() will be called again and it's not the same update
Good point. reworded.


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.h@260
PS10, Line 260: to
> extra to
Done


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc@101
PS10, Line 101: const string STATESTORE_TOTAL_KEY_SIZE_BYTES = 
"statestore.total-key-size-bytes";
Long line.


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc@161
PS10, Line 161: insert(make_pair
> emplace
Done


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc@174
PS10, Line 174: insert(make_pair
> emplace
Done


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc@285
PS10, Line 285: 
GetSubscribedTopics(topic.topic_name)->emplace(topic.topic_name, 
topic.is_transient)
> The copy/move-constructable requirement can be avoided with
I didn't realise that unordered_map could store non-movable elements. Thanks 
for the tip.


http://gerrit.cloudera.org:8080/#/c/9123/10/be/src/statestore/statestore.cc@511
PS10, Line 511: scriber I
> immediately
Done


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@156
PS11, Line 156: const TTopicItem& item: entries
> I think entry and item mean the same thing and should be unified.
I switched "item" to "entry" where it made sense. I didn't rename the 
TTopicItem type.


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@174
PS11, Line 174: insert(make_pai
> emplace
Done


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@298
PS11, Line 298: == true
> remove
yeah that was weird.


http://gerrit.cloudera.org:8080/#/c/9123/11/be/src/statestore/statestore.cc@651
PS11, Line 651:
  :   vector entry_versions = 
topic->Put(update.topic_entries);
  :   if (!subscriber->AddTransientEntries(
  :   update.topic_name, update.topic_entries, 
entry_versions)) {
  : // Subscriber was unregistered - clean up the transient 
entries.
  : for (int i = 0; i < update.topic_entries.size(); ++i) {
  :   topic->DeleteIfVersionsMatch(entry_versions[i], 
update.topic_entries[i].key);
  : }
  :   }
> Why not just hold transient_entry_lock_ and check unregistered_, then put,
I don't necessarily want to hold 'transient_entry_lock_' for the duration of 
Put(), because it mean mean that only one topic can be updated per subscriber 
in parallel.

I didn't see an obvious way to avoid that without adding more fine-grained 
locks.

Would it be better to encapsulate the put-then-revert pattern inside 
AddTransientEntries()?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc49c2d0f2a5bfad822545616b8c62b4b95dc210
Gerrit-Change-Number: 9123
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:42:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

Hello Dimitris Tsirogiannis, Alex Behm, Zach Amsden, Dan Hecht,

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

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

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

Change subject: IMPALA-6204: Remove external DataSource
..

IMPALA-6204: Remove external DataSource

Removes DataSourceScanNode, external data sources, and all affiliated
code, tests, and documentation.

When a data source table is encountered, we now throw an exception. To
the user, this looks like:

  [pannier.ca.cloudera.com:21000] > create table t (x int) stored as textfile 
tblproperties('__IMPALA_DATA_SOURCE_NAME'='V1');
  Query: create table t (x int) stored as textfile 
tblproperties('__IMPALA_DATA_SOURCE_NAME'='V1')
  Fetched 0 row(s) in 0.11s
  [pannier.ca.cloudera.com:21000] > select * from t;
  Query: select * from t
  Query submitted at: 2018-02-01 17:16:26 (Coordinator: 
http://pannier.ca.cloudera.com:25000)
  ERROR: AnalysisException: Failed to load metadata for table: 't'
  CAUSED BY: TableLoadingException: Failed to load metadata for table: 
default.t. Running 'invalidate metadata default.t' may resolve this problem.
  CAUSED BY: UnsupportedOperationException: Eternal Data source table not 
supported.

A test has been added to capture this behavior.

For the most part, I deleted the unused code. In a few places, a renamed
the Thrift enums and threw errors if they're encountered. For Thrift
structs, I left a comment about the now-skipped id that used to
represent a data-source related entry.

Cherry-picks: not for 2.x

Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
---
M CMakeLists.txt
M be/generated-sources/gen-cpp/CMakeLists.txt
M be/src/catalog/catalog-util.cc
M be/src/exec/CMakeLists.txt
M be/src/exec/catalog-op-executor.cc
M be/src/exec/catalog-op-executor.h
D be/src/exec/data-source-scan-node.cc
D be/src/exec/data-source-scan-node.h
M be/src/exec/exec-node.cc
D be/src/exec/external-data-source-executor.cc
D be/src/exec/external-data-source-executor.h
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M be/src/scheduling/scheduler.cc
M be/src/service/client-request-state.cc
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M bin/clean-cmake.sh
M bin/clean.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M common/thrift/Data.thrift
M common/thrift/Descriptors.thrift
D common/thrift/ExternalDataSource.thrift
M common/thrift/Frontend.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
D docs/topics/impala_create_data_source.xml
D docs/topics/impala_data_sources.xml
D docs/topics/impala_drop_data_source.xml
M docs/topics/impala_new_features.xml
M docs/topics/impala_show.xml
D ext-data-source/.gitignore
D ext-data-source/CMakeLists.txt
D ext-data-source/api/pom.xml
D 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/util/SerializationUtils.java
D 
ext-data-source/api/src/main/java/org/apache/impala/extdatasource/v1/ExternalDataSource.java
D ext-data-source/pom.xml
D ext-data-source/sample/pom.xml
D 
ext-data-source/sample/src/main/java/org/apache/impala/extdatasource/sample/EchoDataSource.java
D ext-data-source/test/pom.xml
D 
ext-data-source/test/src/main/java/org/apache/impala/extdatasource/AllTypesDataSource.java
M fe/CMakeLists.txt
M fe/pom.xml
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
D fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
D fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
D fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
D fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
D fe/src/main/java/org/apache/impala/catalog/DataSource.java
D fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
D fe/src/main/java/org/apache/impala/extdatasource/ApiVersion.java
D 
fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
D fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M

[Impala-ASF-CR] IMPALA-6219: Use AES-GCM for spill-to-disk encryption

2018-02-05 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9032 )

Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption
..


Patch Set 15: Code-Review+2

Thanks for fixing it and running the tests Xianda.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24
Gerrit-Change-Number: 9032
Gerrit-PatchSet: 15
Gerrit-Owner: Xianda Ke 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Xianda Ke 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:16:51 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9212 )

Change subject: Bump Kudu version to b315d0e
..


Patch Set 1: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9212
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:11:13 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9212 )

Change subject: Bump Kudu version to b315d0e
..

Bump Kudu version to b315d0e

Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
---
M buildall.sh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Thomas Tauber-Marshall: Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9212
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-6448: Re-enable kerberized testing with KRPC

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9164 )

Change subject: IMPALA-6448: Re-enable kerberized testing with KRPC
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6412978316de90875c98f8fbe51c8d215c227b18
Gerrit-Change-Number: 9164
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:10:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6448: Re-enable kerberized testing with KRPC

2018-02-05 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9164 )

Change subject: IMPALA-6448: Re-enable kerberized testing with KRPC
..


Patch Set 4: Code-Review+2

GVO failed due to an unrelated flaky test. Retrying

Rebase, carry +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6412978316de90875c98f8fbe51c8d215c227b18
Gerrit-Change-Number: 9164
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:10:16 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump Kudu version to b315d0e

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9212 )

Change subject: Bump Kudu version to b315d0e
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9212
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:10:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 4:

(1 comment)

I've been generally hesitant of removing anything from Thrift, from old 
protocol buffer days, but I think you all are right that nothing is serialized, 
so it's safe to remove.

>> How can users identify and purge their HMS after this change?

The table property __IMPALA_DATA_SOURCE_NAME will cause a table to be 
unqueryable. That code is new, simple, and I think has to be in there.

Do you think that's sufficient?

http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:

http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@50
PS4, Line 50: EQ("=", "eq"),
> what happened here?
extdatasources would get a serialized version of operator stuff handed over to 
them for push downs (extdatasource.thrift.TComparisonOp). Since extdatasources 
no longer exist, the mapping between operators and TComparisonOp is no longer 
necessary.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:08:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask

Tim Armstrong has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9214 )

Change subject: Force inlining of BloomFilter::MakeMask
..

Force inlining of BloomFilter::MakeMask

I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.

Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
---
M be/src/util/bloom-filter.h
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
Gerrit-Change-Number: 9214
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong

[Impala-ASF-CR] Force inlining of BloomFilter::MakeMask

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9214


Change subject: Force inlining of BloomFilter::MakeMask
..

Force inlining of BloomFilter::MakeMask

I noticed that this function was showing up in perf top for TPC-H Q8
running locally. It wasn't inlined into BloomFilter::BucketFindAVX2.
Inlining made the query ~5% faster for me locally.

Change-Id: I89282f6c315570bea5ad8a0f854cb6eea0592923
---
M be/src/util/bloom-filter.h
1 file changed, 1 insertion(+), 1 deletion(-)



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

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

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

2018-02-05 Thread Zach Amsden (Code Review)

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9192/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9192/4//COMMIT_MSG@23
PS4, Line 23:   CAUSED BY: UnsupportedOperationException: Eternal Data source 
table not supported.
Eternal Data source?


http://gerrit.cloudera.org:8080/#/c/9192/4//COMMIT_MSG@27
PS4, Line 27: For the most part, I deleted the unused code. In a few places, a 
renamed
I renamed


http://gerrit.cloudera.org:8080/#/c/9192/4/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:

http://gerrit.cloudera.org:8080/#/c/9192/4/common/thrift/CatalogObjects.thrift@38
PS4, Line 38:   DEPRECATED_DATA_SOURCE, // removed in Impala 3.0
I'm not sure I see the point of keeping this around instead of just totally 
deleting it.


http://gerrit.cloudera.org:8080/#/c/9192/4/common/thrift/CatalogObjects.thrift@48
PS4, Line 48:   DEPRECATED_DATA_SOURCE_TABLE, // removed in Impala 3.0
Same comment.


http://gerrit.cloudera.org:8080/#/c/9192/4/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/9192/4/common/thrift/PlanNodes.thrift@43
PS4, Line 43:   DEPRECATED_DATA_SOURCE_NODE, // removed in Impala 3.0
Again, why do we need this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:01:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 4:

(2 comments)

High-level questions:
- Why are we still keeping datasource related enums and code around?
- How can users identify and purge their HMS after this change?

http://gerrit.cloudera.org:8080/#/c/9192/4/be/src/exec/catalog-op-executor.cc
File be/src/exec/catalog-op-executor.cc:

http://gerrit.cloudera.org:8080/#/c/9192/4/be/src/exec/catalog-op-executor.cc@71
PS4, Line 71: } else if (request.ddl_params.ddl_type == 
TDdlType::DEPRECATED_DROP_DATA_SOURCE) {
Why would we keep this around? SQL that might go through this path doesn't even 
parse anymore.

There are a few other places like this where I don't understand why we'd want 
to keep them.


http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:

http://gerrit.cloudera.org:8080/#/c/9192/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@50
PS4, Line 50: EQ("=", "eq"),
what happened here?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:53:07 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9212 )

Change subject: Bump Kudu version to b315d0e
..


Patch Set 1:

This is just so that Impala will be getting developed against a newer version 
of Kudu. There will be another version bump before the next release.

Toolchain build job:
https://master-02.jenkins.cloudera.com/view/Impala/view/Toolchain%20build/job/impala-toolchain-package-build/44/


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9212
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:48:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6449: Use CLOCK MONOTONIC in ConditionVariable

2018-02-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9158 )

Change subject: IMPALA-6449: Use CLOCK_MONOTONIC in ConditionVariable
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I81611cfd5e7c5347203fe7fa6b0f615602257f87
Gerrit-Change-Number: 9158
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:46:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6449: Use CLOCK MONOTONIC in ConditionVariable

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9158 )

Change subject: IMPALA-6449: Use CLOCK_MONOTONIC in ConditionVariable
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9158/1//COMMIT_MSG@13
PS1, Line 13: by switching to
> Makes sense. Let's just add that to the commit message then, so it's useful
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I81611cfd5e7c5347203fe7fa6b0f615602257f87
Gerrit-Change-Number: 9158
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:45:53 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6449: Use CLOCK MONOTONIC in ConditionVariable

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9158 )

Change subject: IMPALA-6449: Use CLOCK_MONOTONIC in ConditionVariable
..


Patch Set 2: Code-Review+2

Carry +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I81611cfd5e7c5347203fe7fa6b0f615602257f87
Gerrit-Change-Number: 9158
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:46:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6449: Use CLOCK MONOTONIC in ConditionVariable

Hello Jim Apple, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6449: Use CLOCK_MONOTONIC in ConditionVariable
..

IMPALA-6449: Use CLOCK_MONOTONIC in ConditionVariable

ConditionVariable is a thin wrapper around pthread_cond_*.
Currently, pthread_cond_timedwait() uses the default attribute
CLOCK_REALTIME. This is susceptible to adjustment to the system
clock from various sources such as NTP and time may go backward.
This change fixes the problem by switching to using CLOCK_MONOTONIC
so time will be monotonic although the frequency of the clock ticks
may still be adjusted by NTP. Ideally, we should use CLOCK_MONOTONIC_RAW
but it's available only on Linux kernel 2.6.28 or latter. This change
also get rids of some usage of boost::get_system_time() which suffers
from the same problem.

Change-Id: I81611cfd5e7c5347203fe7fa6b0f615602257f87
---
M be/src/rpc/thrift-server.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/service/impala-server.cc
M be/src/util/blocking-queue.h
M be/src/util/condition-variable.h
M be/src/util/promise.h
M be/src/util/time.h
7 files changed, 44 insertions(+), 36 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I81611cfd5e7c5347203fe7fa6b0f615602257f87
Gerrit-Change-Number: 9158
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

2018-02-05 Thread Dan Hecht (Code Review)

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 4:

Given that data sources weren't persisted to the HMS, why do we still need the 
various internal enums?  Isn't returning an error on the various DATA SOURCE 
related SQL statements (like CREATE DATA SOURCE) enough, and then everything 
else can be deleted?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:44:14 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump Kudu version to b315d0e

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9212


Change subject: Bump Kudu version to b315d0e
..

Bump Kudu version to b315d0e

Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
---
M buildall.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04e50a3dd79ac68b1d2319a33ade073d6454fbbb
Gerrit-Change-Number: 9212
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-5269: Fix issue with final line of query followed by a comment

2018-02-05 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9191 )

Change subject: IMPALA-5269: Fix issue with final line of query followed by a 
comment
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I54f9a8f65214023520eaa010fc462a663d02d258
Gerrit-Change-Number: 9191
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:28:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9192 )

Change subject: IMPALA-6204: Remove external DataSource
..


Patch Set 4:

https://jenkins.impala.io/job/gerrit-verify-dryrun-external/74/ running. Last 
commit was missing a 1-line "git amend"; whoops.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3a6740466ed7372b71d948c705b30886dcfb6
Gerrit-Change-Number: 9192
Gerrit-PatchSet: 4
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 05 Feb 2018 18:01:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6204: Remove external DataSource