[Impala-ASF-CR] IMPALA-5315: Cast to timestamp fails for YYYY-M-D format

[Vincent Tran (Code Review)]

Hello Lars Volker, Tim Armstrong, Dan Hecht,

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

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

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

Change subject: IMPALA-5315: Cast to timestamp fails for -M-D format
..

IMPALA-5315: Cast to timestamp fails for -M-D format

This change allows casting of a string in 'lazy' date/time
format to timestamp. The supported lazy date formats are:
  -[M]M-[d]d
  -[M]M-[d]d [H]H:[m]m:[s]s[.S]
  -[M]M-[d]dT[H]H:[m]m:[s]s[.S]

We will also take a variety of separators (including "-",
"/", "T", "Z", ":")

We will incur a SCAN performance penalty (approximately 1/2
TotalReadThroughput) when the string is in one of these
lazy date/time format.

Testing:
Benchmarked the performance consequence by executing this SQL on
a private build over 3.8 billion rows:
select min(cast (time_string as timestamp)) from private.impala_5315

Added tests for valid and invalid date/time format strings
in expr-test.cc to be inline with existing tests for CAST() function.

Finally, this change also moves the following two functions to be
adjacent to each other for readability:

Parse(const char* str, int len, boost::gregorian::date* d,
  boost::posix_time::time_duration* t);

Parse(const char* str, int len, const DateTimeFormatContext& dt_ctx,
  boost::gregorian::date* d, boost::posix_time::time_duration* t);

Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
3 files changed, 156 insertions(+), 29 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a184a09d7e7783f04d47588537612c2ecec28f
Gerrit-Change-Number: 7009
Gerrit-PatchSet: 12
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vincent Tran 

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Impala Public Jenkins (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 25 Jan 2018 04:36:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Impala Public Jenkins (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..

KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.

Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Reviewed-on: http://gerrit.cloudera.org:8080/9117
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/9121
Reviewed-by: Lars Volker 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpcz_store.cc
2 files changed, 9 insertions(+), 1 deletion(-)

Approvals:
  Todd Lipcon: Looks good to me, but someone else must approve
  Lars Volker: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 7
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 25 Jan 2018 04:33:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Reviewed-on: http://gerrit.cloudera.org:8080/9062
Reviewed-by: Taras Bobrovytsky 
Tested-by: Impala Public Jenkins
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_mem_usage_scaling.py
M tests/query_test/test_tpcds_queries.py
23 files changed, 235 insertions(+), 139 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 8
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6383: free memory after skipping parquet row groups

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6383: free memory after skipping parquet row groups
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Gerrit-Change-Number: 9059
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 03:55:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6383: free memory after skipping parquet row groups

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6383: free memory after skipping parquet row groups
..

IMPALA-6383: free memory after skipping parquet row groups

Before this patch, resources were only flushed after breaking out of
NextRowGroup(). This is a problem because resources can be allocated
for skipped row groups (e.g. for reading dictionaries).

Testing:
Tested in conjunction with a prototype buffer pool patch that was
DCHECKing before the change.

Added DCHECKs to the current version to ensure the streams are cleared
up as expected.

Ran the repro for IMPALA-6419 to confirm this iteration of the patch
fixed the original problem.

Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Reviewed-on: http://gerrit.cloudera.org:8080/9059
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-parquet-scanner.h
M be/src/exec/scanner-context.h
3 files changed, 44 insertions(+), 21 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Gerrit-Change-Number: 9059
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8914 )

Change subject: IMPALA-6193: Track memory of incoming data streams
..


Patch Set 10:

Please see PS10.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 03:26:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Lars Volker (Code Review)]

Hello Michael Ho, Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6193: Track memory of incoming data streams
..

IMPALA-6193: Track memory of incoming data streams

This change adds memory tracking to incoming transmit data RPCs when
using KRPC. We track memory against a global tracker called "Data Stream
Service" until it is handed over to the stream manager. There we track
it in a global tracker called "Data Stream Queued RPC Calls" until a
receiver registers and takes over the early sender RPCs. Inside the
receiver, memory for deferred RPCs is tracked against the fragment
instance's memtracker until we unpack the batches and add them to the
row batch queue.

The DCHECK in MemTracker::Close() covers that all memory consumed by a
tracker gets release eventually. In addition to that, this change adds a
custom cluster test that makes sure that queued memory gets tracked by
inspecting the peak consumption of the new memtrackers.

Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
---
M be/src/rpc/impala-service-pool.cc
M be/src/rpc/impala-service-pool.h
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/runtime/exec-env.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/mem-tracker.h
M be/src/util/memory-metrics.h
M common/protobuf/data_stream_service.proto
A tests/custom_cluster/test_krpc_mem_usage.py
A tests/verifiers/mem_usage_verifier.py
13 files changed, 313 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8914 )

Change subject: IMPALA-6193: Track memory of incoming data streams
..


Patch Set 9:

(6 comments)

Thanks for the review comments. I'm not particularly happy with the fact that I 
had missed some of the issues. If you have a suggestion how we can increase the 
test coverage to catch those, please let me know.

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc
File be/src/rpc/impala-service-pool.cc:

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@109
PS9, Line 109: c->GetTransferSize()
> Not sure it's safe to assume that c is still valid at this point. The react
Done


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@164
PS9, Line 164: c->GetTransferSize()
> Same here.
Done


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@169
PS9, Line 169: c->GetTransferSize()
> Same here.
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h@126
PS7, Line 126: WARN_UNUSED_RESULT
> I believe we have NODISCARD in the Status class declaration so we no longer
I didn't know we had that. The commit message of the change that introduces it 
mentions that it has no effect on GCC 4.9.2 
(https://gerrit.cloudera.org/c/7253/), so I think we still need 
WARN_UNUSED_RESULT.


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-mgr.cc
File be/src/runtime/krpc-data-stream-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-mgr.cc@122
PS9, Line 122:   for (const unique_ptr& ctx : 
early_senders.closed_sender_ctxs) {
> Shouldn't we also call mem_tracker_->Release(ctx->rpc_context->GetTransferS
Yes, added it. Do you know of a test that exercised this code?


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

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-recvr.cc@407
PS9, Line 407: if (UNLIKELY(!status.ok())) {
> Missing DiscardAndReleaseRpcMemory(ctx->rpc_context); ?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 03:26:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6377: Bump breakpad version to include the fix for Breakpad #752

[Sailesh Mukil (Code Review)]

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

Change subject: IMPALA-6377: Bump breakpad version to include the fix for 
Breakpad #752
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b6212ab77eff2df0cb51339c25183ae4f22b07b
Gerrit-Change-Number: 9131
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:42:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Mostafa Mokhtar (Code Review)]

Mostafa Mokhtar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9125 )

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:36:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6377: Bump breakpad version to include the fix for Breakpad #752

[Lars Volker (Code Review)]

Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9131


Change subject: IMPALA-6377: Bump breakpad version to include the fix for 
Breakpad #752
..

IMPALA-6377: Bump breakpad version to include the fix for Breakpad #752

This change bumps the Breakpad version to pull in the fix for
https://bugs.chromium.org/p/google-breakpad/issues/detail?id=752 .

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



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9b6212ab77eff2df0cb51339c25183ae4f22b07b
Gerrit-Change-Number: 9131
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 3: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:31:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6394: Enable HDFS debug logging

[Tianyi Wang (Code Review)]

Tianyi Wang has abandoned this change. ( http://gerrit.cloudera.org:8080/9082 )

Change subject: IMPALA-6394: Enable HDFS debug logging
..


Abandoned

Data loading is strangely not working with this patch.
--
To view, visit http://gerrit.cloudera.org:8080/9082
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ide09d5abb141dbbb6bc1ee66b69144ac41f841d9
Gerrit-Change-Number: 9082
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-4835: prerequisite buffer pool changes

[Michael Ho (Code Review)]

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

Change subject: IMPALA-4835: prerequisite buffer pool changes
..


Patch Set 2: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9097/2/be/src/runtime/bufferpool/buffer-pool-test.cc
File be/src/runtime/bufferpool/buffer-pool-test.cc:

http://gerrit.cloudera.org:8080/#/c/9097/2/be/src/runtime/bufferpool/buffer-pool-test.cc@767
PS2, Line 767: handle
This refers to BufferHandle, not ClientHandle, right ? If so, please state it 
explicitly.


http://gerrit.cloudera.org:8080/#/c/9097/2/be/src/runtime/bufferpool/buffer-pool-test.cc@797
PS2, Line 797:   }
ASSERT_TRUE(handle.is_open());


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

http://gerrit.cloudera.org:8080/#/c/9097/2/be/src/runtime/bufferpool/buffer-pool.h@231
PS2, Line 231: WARN_UNUSED_RESULT
nit: long line. Not needed ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f2196722df59f2d367787c0550058022e296e24
Gerrit-Change-Number: 9097
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:25:22 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Patch thrift-0.9.3 for python 2.6.

[Tianyi Wang (Code Review)]

Tianyi Wang has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9109 )

Change subject: Patch thrift-0.9.3 for python 2.6.
..

Patch thrift-0.9.3 for python 2.6.

Thrift 0.9.3 doesn't work with python 2.6. The compatibility is broken
by THRIFT-2846 and is fixed as a part of THRIFT-3505. This patch adds
patch 3 to thrift 0.9.3, which is the upstream patch excluding tests and
makefiles. The impala test test_client_ssl broken by thrift-0.9.3-p2 and
python 2.6 now works.

Change-Id: I9a7951e8b99c1d4b866e1d331860e3b82aa3a3e9
---
M buildall.sh
A source/thrift/thrift-0.9.3-patches/0003-THRIFT-3505.patch
2 files changed, 434 insertions(+), 1 deletion(-)

Approvals:
  Tianyi Wang: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9a7951e8b99c1d4b866e1d331860e3b82aa3a3e9
Gerrit-Change-Number: 9109
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 

[native-toolchain-CR] Patch thrift-0.9.3 for python 2.6.

[Tianyi Wang (Code Review)]

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

Change subject: Patch thrift-0.9.3 for python 2.6.
..

Patch thrift-0.9.3 for python 2.6.

Thrift 0.9.3 doesn't work with python 2.6. The compatibility is broken
by THRIFT-2846 and is fixed as a part of THRIFT-3505. This patch adds
patch 3 to thrift 0.9.3, which is the upstream patch excluding tests and
makefiles. The impala test test_client_ssl broken by thrift-0.9.3-p2 and
python 2.6 now works.

Change-Id: I9a7951e8b99c1d4b866e1d331860e3b82aa3a3e9
---
M buildall.sh
A source/thrift/thrift-0.9.3-patches/0003-THRIFT-3505.patch
2 files changed, 434 insertions(+), 1 deletion(-)


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9a7951e8b99c1d4b866e1d331860e3b82aa3a3e9
Gerrit-Change-Number: 9109
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

Tianyi Wang has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9122 )

Change subject: Build old versions of gperftools and thrift.
..

Build old versions of gperftools and thrift.

We recently bumped gperftools and thrift version in the toolchain but
ran into issues when trying to use the new versions in Impala. This
change enabled builds of old versions of both dependencies, so that
we can bump versions of other dependencies.

Based on Lars Volker's patch.

Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
---
M buildall.sh
M source/thrift/build.sh
2 files changed, 7 insertions(+), 3 deletions(-)

Approvals:
  Michael Ho: Looks good to me, but someone else must approve
  Lars Volker: Looks good to me, but someone else must approve
  Tim Armstrong: Looks good to me, approved
  Tianyi Wang: Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2:

The build succeeds on all OSes.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:02:57 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2: Verified+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 02:02:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 7
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:55:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

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

Change subject: IMPALA-4924: Enable Decimal V2 by default
..


Patch Set 7: Code-Review+2

The build was failing with IMPALA-6399. The rebase should fix the issue. 
Forwarding the +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 7
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:55:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6193: Track memory of incoming data streams
..


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc
File be/src/rpc/impala-service-pool.cc:

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@109
PS9, Line 109: c->GetTransferSize()
Not sure it's safe to assume that c is still valid at this point. The reactor 
thread could have sent the reply and deleted InboundCall object already. Seems 
safer to store the transfer size upfront before replying.


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@164
PS9, Line 164: c->GetTransferSize()
Same here.


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/rpc/impala-service-pool.cc@169
PS9, Line 169: c->GetTransferSize()
Same here.


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h@126
PS7, Line 126: WARN_UNUSED_RESULT
> I think we generally try to add this to all functions where it makes sense,
I believe we have NODISCARD in the Status class declaration so we no longer 
have to do it manually for each function which returns Status.


http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-mgr.cc
File be/src/runtime/krpc-data-stream-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-mgr.cc@122
PS9, Line 122:   for (const unique_ptr& ctx : 
early_senders.closed_sender_ctxs) {
Shouldn't we also call 
mem_tracker_->Release(ctx->rpc_context->GetTransferSize()) here too given what 
we did in AddEarlyClosedSender() ?


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

http://gerrit.cloudera.org:8080/#/c/8914/9/be/src/runtime/krpc-data-stream-recvr.cc@407
PS9, Line 407: if (UNLIKELY(!status.ok())) {
Missing DiscardAndReleaseRpcMemory(ctx->rpc_context); ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:54:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_mem_usage_scaling.py
M tests/query_test/test_tpcds_queries.py
23 files changed, 235 insertions(+), 139 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 7
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[native-toolchain-CR] Patch thrift-0.9.3 for python 2.6.

[Philip Zeyliger (Code Review)]

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

Change subject: Patch thrift-0.9.3 for python 2.6.
..


Patch Set 1: Code-Review+2

Looks fine to me.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a7951e8b99c1d4b866e1d331860e3b82aa3a3e9
Gerrit-Change-Number: 9109
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:42:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] InMPALA-4319: remove some deprecated query options

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9118 )

Change subject: InMPALA-4319: remove some deprecated query options
..


Patch Set 1:

(4 comments)

Thank you for working on this.

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

http://gerrit.cloudera.org:8080/#/c/9118/1//COMMIT_MSG@7
PS1, Line 7: InMPALA
typo


http://gerrit.cloudera.org:8080/#/c/9118/1/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/9118/1/be/src/service/query-options.h@a54
PS1, Line 54:
Should we remove this one here, too (IMPALA-2963)?


http://gerrit.cloudera.org:8080/#/c/9118/1/be/src/service/query-options.h@a62
PS1, Line 62:
Can you mention all removed options in the commit message? This way the commit 
will be easier to find in the future.


http://gerrit.cloudera.org:8080/#/c/9118/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

http://gerrit.cloudera.org:8080/#/c/9118/1/common/thrift/ImpalaInternalService.thrift@101
PS1, Line 101:   9: optional bool allow_unsupported_formats = 0
Should we consider compacting the numeric IDs? My understanding is that we 
usually don't support mixing different versions of Impala.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e742e9b0eca0e5c81fd71db3122fef31522fcad
Gerrit-Change-Number: 9118
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:35:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6416: extend Thread::Create to track instance id

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9053 )

Change subject: IMPALA-6416: extend Thread::Create to track instance id
..


Patch Set 1:

(6 comments)

I left some inline comments. I feel it would be better to find a way to 
propagate thread names indefinitely, compared to propagating them for one 
generation (which is my understanding of this patch). Currently it allows us to 
remove the call to the explicit setter in the scan node, but one generation 
further the same mechanism will not work anymore. Maybe we could think of 
deriving names automatically (if they are not explicitly set), e.g. by 
prepending the parent name with an increasing counter. If "Instance Worker Foo" 
creates a child, then its name could be "1 gen by Instance Worker Foo", and if 
this child spawns a thread w/out an explicit name, it could be called "2 gen by 
Instance Worker Foo" or similar. This way we could programatically capture full 
thread trees.

http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/common/thread-debug-info.h
File be/src/common/thread-debug-info.h:

http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/common/thread-debug-info.h@70
PS1, Line 70:   void ExtractInfoFromParent(const ThreadDebugInfo* parent) {
Have you considered storing the parent thread ID (possibly both system and 
boost) in the ThreadDebugInfo, too? While every thread had its own Info it 
seemed superflous to store it again, but for the parent threads it might be 
helpful, especially when thinking of programmatic ways to analyze the thread 
graph.


http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/common/thread-debug-info.h@71
PS1, Line 71: DCHECK(parent != nullptr);
Is there a benefit of not allowing nullptr vs. allowing and ignoring it? The 
latter could make the calling code a bit more concise, but I may be missing 
something.

If you keep the requirement that parent != nullptr, you could consider making 
this a static factory that creates and pre-populates a TDI.


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

http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/exec/hdfs-scan-node.cc@a353
PS1, Line 353: 
My understanding is that this is no longer needed because the information is 
now kept in the parent TDI. Is this correct?


http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/util/thread.h
File be/src/util/thread.h:

http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/util/thread.h@183
PS1, Line 183:   static void SuperviseThread(const string& name, const string& 
category,
Our convention is usually to keep the std:: namespace prefix in header files 
but drop it in .cc files.


http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/util/thread.h@185
PS1, Line 185: parent_thread_info
The comment should explain what this parameter does (and whether it can be 
null), especially since we usually pass input parameters as const& and output 
parameters as non-const ptrs.


http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/util/thread.cc
File be/src/util/thread.cc:

http://gerrit.cloudera.org:8080/#/c/9053/1/be/src/util/thread.cc@350
PS1, Line 350:   if (parent_thread_info) 
thread_debug_info.ExtractInfoFromParent(parent_thread_info);
We usually make the comparison to != nullptr explicit to make it more clear 
what is happening.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27de2962cf0b224c17b685d77dcba3bf2e9db187
Gerrit-Change-Number: 9053
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:24:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6383: free memory after skipping parquet row groups

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6383: free memory after skipping parquet row groups
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Gerrit-Change-Number: 9059
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:12:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Philip Zeyliger (Code Review)]

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

Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9124/1//COMMIT_MSG@14
PS1, Line 14: Testing: With an Impala built with a local override to use
> I think I can "magically" fix this.  We're going to need the property name
Lol. Yeah, that would do it. Be sure to write the assert that says it's one of 
the two things it's allowed to be.

Also, I just learned that JVM8 shipped an unusual scripting engine:

$/usr/lib/jvm/java-8-openjdk-amd64/bin/jrunscript -cp $CLASSPATH -e 
'print("org.apache.hadoop.hive.hbase.HBaseSerDe.HBASE_TABLE_NAME")'
org.apache.hadoop.hive.hbase.HBaseSerDe.HBASE_TABLE_NAME



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:12:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4835: Part 2: Allocate scan range buffers upfront

[Tianyi Wang (Code Review)]

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

Change subject: IMPALA-4835: Part 2: Allocate scan range buffers upfront
..


Patch Set 18:

(8 comments)

I'm still trying to understand the error propagation and cancellation part. 
Here are some comments and questions.

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr-test.cc
File be/src/runtime/io/disk-io-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr-test.cc@1068
PS18, Line 1068: if (needs_buffers) {
Doesn't BufferOpts::ReadInto(client_buffer.data(), SCAN_LEN) provides buffer 
already?


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.h@147
PS18, Line 147:
An extra space


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.h@271
PS18, Line 271: AddBuffersToRange
AllocateBuffersForRange?


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.h@271
PS18, Line 271:   /// If 'needs_buffers' is set to true, the caller must then 
call AddBuffersToRange() to
Can we merge "the caller must then call..." work into this function and remove 
this contract?


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.cc
File be/src/runtime/io/disk-io-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/disk-io-mgr.cc@366
PS18, Line 366: Status DiskIoMgr::AddScanRange(RequestContext* reader, 
ScanRange* range) {
This function is unused.


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/request-context.h
File be/src/runtime/io/request-context.h:

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/io/request-context.h@205
PS18, Line 205:   RequestRange* range, bool schedule_later, bool 
schedule_immediately);
I think combining schedule_later and schedule_immediately into one enum would 
be more clear. e.g. enum { SCHEDULE_NOW, SCHEDULE_UPON_GETNEXT, 
SCHEDULE_BY_CALLER }


http://gerrit.cloudera.org:8080/#/c/8707/17/be/src/runtime/io/request-ranges.h
File be/src/runtime/io/request-ranges.h:

http://gerrit.cloudera.org:8080/#/c/8707/17/be/src/runtime/io/request-ranges.h@309
PS17, Line 309: that
extra 'that'


http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/8707/18/be/src/runtime/tmp-file-mgr.cc@558
PS18, Line 558: // The write will not be in flight if we returned with an 
error.
Does the in_flight here have the same meaning as 
RequestContext::in_flight_ranges?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia243bf8b62feeea602b122e0503ea4ba7d3ee70f
Gerrit-Change-Number: 8707
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jan 2018 00:05:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Mostafa Mokhtar,

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

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

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..

IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

This change bumps the threshold of RPC duration above which a RPC
is logged. It's increased from 1 second to 2 minutes which is
a conservative value in order to reduce the amount of logging from
RpczStore::LogTrace() when an Impala demon is busy.

Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
---
M be/src/rpc/rpc-mgr.cc
1 file changed, 6 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9125/1/be/src/rpc/rpc-mgr.cc@60
PS1, Line 60:   FLAGS_rpc_duration_too_long_ms = 15 * 60 * 1000;
> 2 minutes should be reasonable enough to capture issues and not spew the lo
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:50:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc
File be/src/kudu/rpc/rpcz_store.cc:

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc@44
PS2, Line 44:  "Threshold (in milliseconds) above which a RPC is 
considered too long and its "
> nit: The lines look too long in Gerrit.
Apparently, the Kudu codebase doesn't seem to limit to 90 characters per line 
for some reasons. Keeping it the same to avoid future conflicts.


http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc@45
PS2, Line 45: The time measured is between "
:  "when a RPC is accepted and when its call handler 
completes
> nit: The time is measured from when an RPC is accepted until its call handl
Keeping it the same to avoid future conflicts.

I can push another change on Kudu side to fix up the wording if you think it's 
worth it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:44:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Philip Zeyliger (Code Review)]

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

Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..


Patch Set 1:

(4 comments)

Minor feedback. (Code logic itself looks fine.)

I'm a little worried about the fact that we've published instructions on how to 
use "hbase.table.name" and that this might complicate things.

Are you planning to change "hbase.table.name" references in various 
dataload-related scripts? (I'd be cool with that as a follow-on.)

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

http://gerrit.cloudera.org:8080/#/c/9124/1//COMMIT_MSG@9
PS1, Line 9: Upstream Hive has changed the property name used to specify
I think also HBase? Can we pull in the JIRA numbers in the corresponding 
projects?


http://gerrit.cloudera.org:8080/#/c/9124/1//COMMIT_MSG@10
PS1, Line 10: HBase table names.  It is relatively straitforward to make
nit: straightforward


http://gerrit.cloudera.org:8080/#/c/9124/1//COMMIT_MSG@14
PS1, Line 14: Testing: With an Impala built with a local override to use
The documentation in docs/topics/impala_hbase.xml refers "hbase.table.name" and 
also needs to be fixed. (Or maybe Hive shouldn't have changed this.) Is a 
simpler reproduction to create a table from Hive and then just query it here?


http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
File fe/src/main/java/org/apache/impala/catalog/HBaseTable.java:

http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@450
PS1, Line 450: // the original table name, and if that fails, try looking 
for the
As Dimitris points out, this would be clearer if we said:

We look up the HBase Table name in the following order:
  * Try looking up HBASE_TABLE_NAME_KEY in SERDEPROPERTIES; return it in found.
  * Try looking up HBASE_TABLE_NAME_COMPAT_KEY in SERDEPROPERTIES; return it if 
found.
  * If the table is in the default DB, return the table name.
  * Return db.tablename.

My point is that I'm suggesting we rename HBASE_TABLE_NAME to 
HBASE_TABLE_NAME_KEY, since it's the key in SERDEPROPERTIES. By my look, the 
only users are here in this file, so it's not a hard change. (If it was 
pervasive, I wouldn't be bothered if we kept it the way it is.)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:37:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Zach Amsden (Code Review)]

Hello Philip Zeyliger, Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..

IMPALA-6440: Backwards compatibility for HBase metadata

Upstream Hive has changed the property name used to specify
HBase table names.  It is relatively straitforward to make
this backwards compatible, no matter what version of Hive
Impala is linked with.

Testing: With an Impala built with a local override to use
latest Hive-2.1.1, perform a full data load, then:

  create external table test_compute_stats like
functional_hbase.alltypes;
  select * from test_compute_stats;

Before, this failed with TableNotFound, now this succeeds.

Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
---
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
1 file changed, 15 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6383: free memory after skipping parquet row groups

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9059 )

Change subject: IMPALA-6383: free memory after skipping parquet row groups
..


Patch Set 4: Code-Review+2

Checked with Alex that he has no further comments.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Gerrit-Change-Number: 9059
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:32:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Zach Amsden (Code Review)]

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

Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
File fe/src/main/java/org/apache/impala/catalog/HBaseTable.java:

http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@117
PS1, Line 117: for backwards compatibility with the original spec.
> Actually, I would mention here which was the latest version to support this
Done


http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@449
PS1, Line 449: try
 : // the original table name, and if that fails, try looking 
for the
 : // original table name property in SERDEPROPERTIES.
> I am a bit confused with the terminology. Is HBaseSerDe.HBASE_TABLE_NAME th
The original table name, and the name that used to be stored in SERDEPROPERTIES 
is "hbase.table.name".

HBaseSerDe.HBASE_TABLE_NAME is the new table name, which varies depending on 
which version of HBase we link with (and varies even with versions due to late 
breaking changes).


http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@452
PS1, Line 452: look for
> nit: we're not looking for it, we simply construct it, no?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:29:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9121 )

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 2: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc
File be/src/kudu/rpc/rpcz_store.cc:

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc@44
PS2, Line 44:  "Threshold (in milliseconds) above which a RPC is 
considered too long and its "
nit: The lines look too long in Gerrit.


http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc@45
PS2, Line 45: The time measured is between "
:  "when a RPC is accepted and when its call handler 
completes
nit: The time is measured from when an RPC is accepted until its call handler 
completes.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:25:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc
File be/src/kudu/rpc/rpcz_store.cc:

http://gerrit.cloudera.org:8080/#/c/9121/2/be/src/kudu/rpc/rpcz_store.cc@43
PS2, Line 43: DEFINE_int32_hidden
Modification from original patch. Hide this flag by default in Impala.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:19:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

Hello Lars Volker, Kudu Jenkins, Sailesh Mukil, Todd Lipcon,

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

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

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..

KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.

Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Reviewed-on: http://gerrit.cloudera.org:8080/9117
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpcz_store.cc
2 files changed, 9 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8914 )

Change subject: IMPALA-6193: Track memory of incoming data streams
..


Patch Set 9:

(13 comments)

Thanks for the comments. Please see my replies inline and PS9, which also 
contains a test for clearing out deferred RPCs. This was a TODO left from Tim's 
review.

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/impala-service-pool.cc
File be/src/rpc/impala-service-pool.cc:

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/impala-service-pool.cc@161
PS7, Line 161:   if (queue_status == kudu::rpc::QueueStatus::QUEUE_SHUTDOWN) {
> We may reach here when trying to enqueue after the shutdown of the queue ha
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/impala-service-pool.cc@164
PS7, Line 164: mem_tracker_->Release(c->GetTransferSize());
> Should we release the memory in MemTracker here too ?
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/rpc/rpc-mgr.h@126
PS7, Line 126: WARN_UNUSED_RESULT
> Not needed ?
I think we generally try to add this to all functions where it makes sense, and 
I couldn't think of a case where we want to ignore a failure here. Can you give 
an example?


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

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/exec-env.cc@343
PS7, Line 343: // Bump thread cache to 1GB to reduce contention for 
TCMalloc central
 : // list's spinlock.
 : if (FLAGS_tcmalloc_max_total_thread_cache_bytes == 0) {
 :   FLAGS_tcmalloc_max_total_thread_cache_bytes = 1 << 30;
 : }
> These lines should be gone after rebase.
Thanks for the heads up, I'll double check.


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

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@293
PS7, Line 293: //
> nit: ///
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@294
PS7, Line 294:   /// specific receiver. Not owned.
> Not owned.
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@295
PS7, Line 295:   MemTracker* mem_tracker_ = nullptr;
> = nullptr;
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@298
PS7, Line 298:   /// requests. Memory for new incoming requests is initially 
tracked against this tracker
> Not owned.
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@297
PS7, Line 297:  MemTracker which is used by the DataStreamService to track 
memory for incoming
 :   /// requests. Memory for new incoming requests is initially 
tracked against this tr
> It's not very clear from the comments what the relationship between mem_tra
Yes, I tried to make the comment more clear.


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@299
PS7, Line 299:   /// before the requests are handed over to the service. It is 
the services' (here: this
> = nullptr;
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@481
PS7, Line 481:   void Maintenance();
> Please add a comment on when this is used. Please also state the side effec
Done


http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-mgr.h@485
PS7, Line 485:   /// tracked against 'mem_tracker_'. After calling this 
function, the inbound sidecars of
> Please add a comment on when this is used.
Done


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

http://gerrit.cloudera.org:8080/#/c/8914/7/be/src/runtime/krpc-data-stream-recvr.cc@130
PS7, Line 130:   /// MemTracker attached to 'recvr_'. After calling this 
function, the inbound sidecars
> Please add that it's no longer safe to access the inbound sidecars after th
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:09:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6193: Track memory of incoming data streams

[Lars Volker (Code Review)]

Hello Michael Ho, Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6193: Track memory of incoming data streams
..

IMPALA-6193: Track memory of incoming data streams

This change adds memory tracking to incoming transmit data RPCs when
using KRPC. We track memory against a global tracker called "Data Stream
Service" until it is handed over to the stream manager. There we track
it in a global tracker called "Data Stream Queued RPC Calls" until a
receiver registers and takes over the early sender RPCs. Inside the
receiver, memory for deferred RPCs is tracked against the fragment
instance's memtracker until we unpack the batches and add them to the
row batch queue.

The DCHECK in MemTracker::Close() covers that all memory consumed by a
tracker gets release eventually. In addition to that, this change adds a
custom cluster test that makes sure that queued memory gets tracked by
inspecting the peak consumption of the new memtrackers.

Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
---
M be/src/rpc/impala-service-pool.cc
M be/src/rpc/impala-service-pool.h
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/runtime/exec-env.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/mem-tracker.h
M be/src/util/memory-metrics.h
M common/protobuf/data_stream_service.proto
A tests/custom_cluster/test_krpc_mem_usage.py
A tests/verifiers/mem_usage_verifier.py
13 files changed, 307 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2df1204d2483313a8a18e5e3be6cec9e402614c4
Gerrit-Change-Number: 8914
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9125/1/be/src/rpc/rpc-mgr.cc@47
PS1, Line 47: DECLARE_int32(rpc_duration_too_long_ms);
> Can you add a comment saying that this is a KRPC flag?
Done


http://gerrit.cloudera.org:8080/#/c/9125/1/be/src/rpc/rpc-mgr.cc@60
PS1, Line 60:   FLAGS_rpc_duration_too_long_ms = 15 * 60 * 1000;
> I think the default of 15 minutes is too long, since we have noticed querie
Do you think 5 or 10 minutes would be more reasonable ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 23:04:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

Hello Sailesh Mukil, Mostafa Mokhtar,

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

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

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..

IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

This change bumps the threshold of RPC duration above which a RPC
is logged. It's increased from 1 second to 15 minutes which is
a conservative value in order to reduce the amount of logging from
RpczStore::LogTrace() when an Impala demon is busy.

Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
---
M be/src/rpc/rpc-mgr.cc
1 file changed, 6 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Mostafa Mokhtar (Code Review)]

Mostafa Mokhtar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9125 )

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9125/1/be/src/rpc/rpc-mgr.cc@60
PS1, Line 60:   FLAGS_rpc_duration_too_long_ms = 15 * 60 * 1000;
I think the default of 15 minutes is too long, since we have noticed queries 
taking that long to fail in case of network partitioning.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:55:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 15: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 15
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:47:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9124 )

Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
File fe/src/main/java/org/apache/impala/catalog/HBaseTable.java:

http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@117
PS1, Line 117: for backwards compatibility with the original spec.
Actually, I would mention here which was the latest version to support this 
name.


http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@449
PS1, Line 449: try
 : // the original table name, and if that fails, try looking 
for the
 : // original table name property in SERDEPROPERTIES.
I am a bit confused with the terminology. Is HBaseSerDe.HBASE_TABLE_NAME the 
'original table name' or the 'original table name property is SERDEPROPERTIES'?


http://gerrit.cloudera.org:8080/#/c/9124/1/fe/src/main/java/org/apache/impala/catalog/HBaseTable.java@452
PS1, Line 452: look for
nit: we're not looking for it, we simply construct it, no?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:46:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5191: Standardize column alias behavior
..

IMPALA-5191: Standardize column alias behavior

We should not perform alias substitution in the
subexpressions of GROUP BY, HAVING, and ORDER BY
to be more standard conformant.

=== Allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY x;

SELECT NOT bool_col AS nb
FROM functional.alltypes
GROUP BY nb
HAVING nb;

=== Not allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x / 2;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY -x;

SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x
HAVING x > 3;

Some extra checks were added to AnalyzeExprsTest.java.

I had to update other tests to make them pass
since the new behavior is more restrictive.

I added alias.test to the end-to-end tests.

Cherry-picks: not for 2.x.

Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Reviewed-on: http://gerrit.cloudera.org:8080/8801
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
A testdata/workloads/functional-query/queries/QueryTest/alias.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
M 
testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
M tests/query_test/test_queries.py
16 files changed, 252 insertions(+), 60 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 16
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Sailesh Mukil (Code Review)]

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

Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..


Patch Set 1: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/9125/1/be/src/rpc/rpc-mgr.cc@47
PS1, Line 47: DECLARE_int32(rpc_duration_too_long_ms);
Can you add a comment saying that this is a KRPC flag?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:45:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

[Michael Ho (Code Review)]

Michael Ho has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9125


Change subject: IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()
..

IMPALA-6356: Reduce amount of logging from RpczStore::LogTrace()

This change bumps the threshold of RPC duration above which a RPC
is logged. It's increased from 1 second to 15 minutes which is
a conservative value in order to reduce the amount of logging from
RpczStore::LogTrace() when an Impala demon is busy.

Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
---
M be/src/rpc/rpc-mgr.cc
1 file changed, 4 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I347b0dea641368e10ba84bc40ec250c26a4f43b2
Gerrit-Change-Number: 9125
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 

[Impala-ASF-CR] IMPALA-6440: Backwards compatibility for HBase metadata

[Zach Amsden (Code Review)]

Zach Amsden has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9124


Change subject: IMPALA-6440: Backwards compatibility for HBase metadata
..

IMPALA-6440: Backwards compatibility for HBase metadata

Upstream Hive has changed the property name used to specify
HBase table names.  It is relatively straitforward to make
this backwards compatible, no matter what version of Hive
Impala is linked with.

Testing: With an Impala built with a local override to use
latest Hive-2.1.1, perform a full data load, then:

  create external table test_compute_stats like
functional_hbase.alltypes;
  select * from test_compute_stats;

Before, this failed with TableNotFound, now this succeeds.

Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
---
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
1 file changed, 14 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7c5b62f38be275762771ecf39cf898f06f8138b
Gerrit-Change-Number: 9124
Gerrit-PatchSet: 1
Gerrit-Owner: Zach Amsden 

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tim Armstrong (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 22:16:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5990: End-to-end compression of metadata

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5990: End-to-end compression of metadata
..


Patch Set 6:

(14 comments)

I'm pretty happy with this change, probably good to get another pair of eyes. 
Let's ask Dimitris to review.

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-server.h
File be/src/catalog/catalog-server.h:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-server.h@78
PS6, Line 78:   void AddPendingTopicItem(std::string key, const uint8_t* 
topic_data, uint32_t size,
topic_data -> item_data

(the topic is a container of items, and we are passing the data of a single 
item)


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util-test.cc
File be/src/catalog/catalog-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util-test.cc@40
PS6, Line 40:   for (int i = 0; i < 2000 * 1024 * 1024; ++i) { // almost 2GB
* reserve() the size
* why almost 2GB? is there any significance to 2GB? add comment


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h
File be/src/catalog/catalog-util.h:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@36
PS6, Line 36: public:
space before private/protected/public


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@45
PS6, Line 45:   /// will be skipped and the catalog update will still proceed.
will be skipped and the next valid object is returned.


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@46
PS6, Line 46:   virtual jobject next(JNIEnv* env) = 0;
Better to add a virtual destructor


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.h@54
PS6, Line 54:   static jclass pair_class;
nit: use common abbreviations for JNI stuff, "cl" for "class" and "ctor" for 
"constructor", i.e.:

pair_cl, pair_ctor, boolean_cl, boolean_ctor


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc@106
PS6, Line 106: pos_ += 1;
++pos


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/catalog/catalog-util.cc@118
PS6, Line 118:
remove newline


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@428
PS6, Line 428: JNIEXPORT void JNICALL
I'm thinking this function should return a bool for success/failure. That way 
the caller can decide whether what to do with this issue. I think we should 
probably log something and move on in most cases.


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@435
PS6, Line 435: // Throw any exception into the frontend
Comment seems wrong, the FE won't see an exception.


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@455
PS6, Line 455: JNIEXPORT void JNICALL
Same comment about returning bool.


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/service/fe-support.cc@468
PS6, Line 468: JNIEXPORT void JNICALL
Same comment about returning bool.


http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/util/jni-util.h
File be/src/util/jni-util.h:

http://gerrit.cloudera.org:8080/#/c/8825/6/be/src/util/jni-util.h@172
PS6, Line 172: public:
single space before public/private


http://gerrit.cloudera.org:8080/#/c/8825/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/8825/6/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@404
PS6, Line 404: {
move to prev line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a8819cad734b3a416eef6c954e55b73cc6023ae
Gerrit-Change-Number: 8825
Gerrit-PatchSet: 6
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 24 Jan 2018 21:56:14 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9122 )

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2: Code-Review+1

Thanks for providing a patch. I kicked of a toolchain build to test the 
resulting artifacts.

Tim, Phil, do you want to have a quick look, too?


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 21:49:49 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Michael Ho (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 21:42:05 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9122/1/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/9122/1/buildall.sh@180
PS1, Line 180: fi
> Should this line be removed now ?
Done



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 21:34:23 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..

Build old versions of gperftools and thrift.

We recently bumped gperftools and thrift version in the toolchain but
ran into issues when trying to use the new versions in Impala. This
change enabled builds of old versions of both dependencies, so that
we can bump versions of other dependencies.

Based on Lars Volker's patch.

Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
---
M buildall.sh
M source/thrift/build.sh
2 files changed, 7 insertions(+), 3 deletions(-)


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Michael Ho (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9122/1/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/9122/1/buildall.sh@180
PS1, Line 180:   GPERFTOOLS_VERSION=2.5 $SOURCE_DIR/source/gperftools/build.sh
Should this line be removed now ?



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 21:26:24 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

Thrift doesn't build with this patch. Please go to 
https://gerrit.cloudera.org/c/9122/


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I119f36135c982e88c07ae626976bec05194a49aa
Gerrit-Change-Number: 9120
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:28:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

[Vuk Ercegovac (Code Review)]

Hello Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6215: Removes race when using LibCache.
..

IMPALA-6215: Removes race when using LibCache.

LibCache's api to provide access to locally cached files has a race.
Currently, the client of the cache accesses the locally cached path
as a string, but nothing guarantees that the associated file is not
removed before the client is done using it. This race is suspected
as the root cause for the flakiness seen in IMPALA-6092. These tests
fail once in a while with classloader errors unable to load java udf
classes. In these tests, the lib cache makes no guarantee that the path
to the jar will remain valid from the time the path is acquired through
the time needed to fetch the jar and resolve the needed classes.

LibCache offers liveness guarantees for shared objects via reference
counting. The fix in this patch extends this API to also cover paths
to locally cached files.

Testing:
- added a test to test_udfs.py that does many concurrent udf uses and
  removals. By increasing the concurrent operations to 100, the issue
  in IMPALA-6092 is locally reproducible on every run. With this fix,
  the problem is no longer reproducible with this test.

Change-Id: I9175085201fe8b11424ab8f88d7b992cb7b0daea
---
M be/src/codegen/llvm-codegen.cc
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M tests/query_test/test_udfs.py
8 files changed, 167 insertions(+), 52 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9175085201fe8b11424ab8f88d7b992cb7b0daea
Gerrit-Change-Number: 9089
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9089 )

Change subject: IMPALA-6215: Removes race when using LibCache.
..


Patch Set 3:

(1 comment)

jira for further cleanup: IMPALA-6439

http://gerrit.cloudera.org:8080/#/c/9089/3/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/9089/3/tests/query_test/test_udfs.py@440
PS3, Line 440:
> nit: extra space at the end, look for red boxes
whoops.. missed these.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9175085201fe8b11424ab8f88d7b992cb7b0daea
Gerrit-Change-Number: 9089
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:27:14 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9122


Change subject: Build old versions of gperftools and thrift.
..

Build old versions of gperftools and thrift.

We recently bumped gperftools and thrift version in the toolchain but
ran into issues when trying to use the new versions in Impala. This
change enabled builds of old versions of both dependencies, so that
we can bump versions of other dependencies.

Based on Lars Volker's patch.

Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
---
M buildall.sh
M source/thrift/build.sh
2 files changed, 7 insertions(+), 2 deletions(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee6c2a13dd288d0f7a8852c7100ec84c9c2e0ba3
Gerrit-Change-Number: 9122
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9121/1/be/src/kudu/rpc/rpc-test.cc
File be/src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/9121/1/be/src/kudu/rpc/rpc-test.cc@46
PS1, Line 46: DECLARE_int32(rpc_duration_too_long_ms);
Not needed. Forgot to remove after the test was removed. Anyhow, keeping it for 
now.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:22:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

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

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Patch Set 1:

Clean cherry-pick.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 20:19:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

Michael Ho has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9121 )

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/9121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

Michael Ho has removed Todd Lipcon from this change.  ( 
http://gerrit.cloudera.org:8080/9121 )

Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..


Removed reviewer Todd Lipcon.
--
To view, visit http://gerrit.cloudera.org:8080/9121
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 

[Impala-ASF-CR] KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

[Michael Ho (Code Review)]

Hello Kudu Jenkins, Todd Lipcon,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: KUDU-2270: Add a flag to control logging in 
RpczStore::LogTrace()
..

KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.

Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Reviewed-on: http://gerrit.cloudera.org:8080/9117
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/rpc/rpcz_store.cc
2 files changed, 9 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Gerrit-Change-Number: 9121
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] [DOCS] Doc for MURMUR HASH() function

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9031 )

Change subject: [DOCS] Doc for MURMUR_HASH() function
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/9031/1//COMMIT_MSG@7
PS1, Line 7: [DOCS]
Let's include a reference to the code JIRA:

IMPALA-3651: [DOCS] ...


http://gerrit.cloudera.org:8080/#/c/9031/1/docs/topics/impala_math_functions.xml
File docs/topics/impala_math_functions.xml:

http://gerrit.cloudera.org:8080/#/c/9031/1/docs/topics/impala_math_functions.xml@862
PS1, Line 862: 2.12.0
rev="IMPALA-3651 2.12.0"

The rev= field is free-form and can take multiple space-separated values. When 
practical, we record both the the code JIRA and the associated release.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I784a5a080d7d13192aac2fca67f841d2d19fc99b
Gerrit-Change-Number: 9031
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:55:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6346: Potential deadlock in KrpcDataStreamMgr

[Michael Ho (Code Review)]

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

Change subject: IMPALA-6346: Potential deadlock in KrpcDataStreamMgr
..


Patch Set 6:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@77
PS6, Line 77: DEFINE_int32(port, 20001, "port on which to run Impala Thrift 
based test backend.");
Why don't we just use FLAGS_be_port here ?


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@79
PS6, Line 79: DECLARE_int32(datastream_service_num_svc_threads);
: DECLARE_int32(datastream_service_queue_depth);
delete ?


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@545
PS6, Line 545: FLAGS_port
FLAGS_krpc_port ?


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@609
PS6, Line 609:   EXPECT_OK(static_cast(
 :   sender.get())->Init(vector({output_exprs}), 
sink, ));
Can this be factored out too ?


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@658
PS6, Line 658: class DataStreamTestThriftOnly : public DataStreamTest {
Please add a comment what this class is for.


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@668
PS6, Line 668:
 : class DataStreamTestForImpala6346 : public DataStreamTest {
Please add a comment why we need to have a class for this test.


http://gerrit.cloudera.org:8080/#/c/8950/6/be/src/runtime/data-stream-test.cc@808
PS6, Line 808: TEST_P(DataStreamTestForImpala6346, TestNoDeadlock) {
Just double checking: this test is able to reproduce the deadlock without the 
fix, right ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7d1a8f12a4821092ca61ccc8a6f20c0404d56c7
Gerrit-Change-Number: 8950
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:50:17 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9120 )

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

> Patch Set 1:
>
> (1 comment)

My bad, I picked the wrong version. I will update it in a subsequent PS once we 
know whether to revert the thrift change altogether.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I119f36135c982e88c07ae626976bec05194a49aa
Gerrit-Change-Number: 9120
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:34:58 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9120 )

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

> Patch Set 1:
>
> Did you tried building thrift 0.9.0-p11? I changed source/thrift/build.sh in 
> the thrift upgrade patch and 0.9.0-p11 doesn't build on my machine now.

Oh, I hadn't tried building it. In that case we should probably revert the 
original change for now. Do the older versions still build? If not, we should 
probably remove them from buildall.sh altogether.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I119f36135c982e88c07ae626976bec05194a49aa
Gerrit-Change-Number: 9120
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:34:26 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Tianyi Wang (Code Review)]

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

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

Did you tried building thrift 0.9.0-p11? I changed source/thrift/build.sh in 
the thrift upgrade patch and 0.9.0-p11 doesn't build on my machine now.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I119f36135c982e88c07ae626976bec05194a49aa
Gerrit-Change-Number: 9120
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:22:53 +
Gerrit-HasComments: No

[native-toolchain-CR] Build old versions of gperftools and thrift.

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9120 )

Change subject: Build old versions of gperftools and thrift.
..


Patch Set 1:

Tim, Phil, adding you as reviewers of one of the changes. Please let me know if 
you prefer to revert one/both of the changes?


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I119f36135c982e88c07ae626976bec05194a49aa
Gerrit-Change-Number: 9120
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:22:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 15: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 15
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 24 Jan 2018 19:10:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5931: Generates scan ranges in planner for s3/adls

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8523 )

Change subject: IMPALA-5931: Generates scan ranges in planner for s3/adls
..


Patch Set 8: Code-Review+1

carry +1 from dimitris


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 8
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 24 Jan 2018 18:29:58 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

[Philip Zeyliger (Code Review)]

Hello Gabor Kaszab, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6318: Revert "Adjustment for hanging query cancellation 
test"
..

IMPALA-6318: Revert "Adjustment for hanging query cancellation test"

Jenkins jobs occasionally hang on test_query_cancellation_during_fetch.
There was a workaround proposal submitted under this Jira ID, however,
apparently jobs still hang on this test randomly. Reverting the
workaround and skipping the test until further fix proposal provided.

This reverts commit 7810d1f9a2c7d59b4b916d4d1793672cd8c33143.

Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Reviewed-on: http://gerrit.cloudera.org:8080/8972
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
M tests/shell/test_shell_commandline.py
M tests/shell/util.py
2 files changed, 11 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51acee49b5a17c4852410b7568fd1d092b114a6d
Gerrit-Change-Number: 9100
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6429: Fix decimal division

[Zach Amsden (Code Review)]

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

Change subject: IMPALA-6429: Fix decimal division
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/exprs/expr-test.cc@2400
PS1, Line 2400:  { true, false, 0, 38, 6 }}},
Suggest also adding another test with full precision decimal(38,3) / 
decimal(1,0), which will overflow in V2 since the result can't fit in 
decimal(38,6).  Even the value 1.0 will work as a divisor for this.


http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/runtime/decimal-value.inline.h@235
PS1, Line 235:   return DecimalUtil::SafeMultiply(left, mult, *overflow) + 
right;
This is going to check *overflow with a branch anyway.  Why not hoist the check 
of *overflow to this function?  Then if (UNLIKELY(*overflow == true,)) return 
garbage, and always call DecimalUtil::SafeMultiply with false.


http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/runtime/decimal-value.inline.h@288
PS1, Line 288:   return DecimalUtil::SafeMultiply(left, mult, *overflow) + 
right;
Same comment applies.


http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/runtime/decimal-value.inline.h@457
PS1, Line 457: bool may_overflow = scale_by > 38;
You could add a leading zero check as well, but that requires knowing the 
maximum value of scale_by.  This adds one more corner case that relies on the 
exact formula for result_scale, and that makes me uncomfortable.  In the 
future, we may want to improve decimal results by allowing the query to specify 
the output scale (as a cast, which gets pushed down into result_scale), and 
doing that is already a bit dangerous as I think there are other places that 
rely on the result scale formulas being predefined.


http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/util/decimal-util.h
File be/src/util/decimal-util.h:

http://gerrit.cloudera.org:8080/#/c/9114/1/be/src/util/decimal-util.h@67
PS1, Line 67: if (UNLIKELY(result / multiplier != v)) *overflow = true;
You can infer !may_overflow from overflow == nullptr, and this should inline 
just as well in cases where constant nullptr is passed.  (Edit - I see the way 
you are using it, this isn't quite true - I think if you adopt my suggestions 
for Add / SubtractLarge, then this may follow).

Also, this is likely expensive for int256_t.  I don't think there is a 
practical way to make that faster without direct access to the underlying type 
(to get the number of leading zeroes).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd1075d9c78986cd975dd29c1125d71ba6560c23
Gerrit-Change-Number: 9114
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Wed, 24 Jan 2018 18:16:28 +
Gerrit-HasComments: Yes

[native-toolchain-CR] Bump Kudu version to c6beta-impala-toolchain-tag1

[Thomas Tauber-Marshall (Code Review)]

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

Change subject: Bump Kudu version to c6beta-impala-toolchain-tag1
..


Patch Set 2:

Turns out, the Jenkins job didn't need to be modified, I just needed to add the 
KUDU_GITHUB_URL param to this review.

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


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
Gerrit-Change-Number: 9086
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Wed, 24 Jan 2018 18:15:07 +
Gerrit-HasComments: No

[native-toolchain-CR] Bump Kudu version to c6beta-impala-toolchain-tag1

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9086 )

Change subject: Bump Kudu version to c6beta-impala-toolchain-tag1
..

Bump Kudu version to c6beta-impala-toolchain-tag1

Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
---
M buildall.sh
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I409cf30dc1554630bdebca1e831c22fc36e024d0
Gerrit-Change-Number: 9086
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Thomas Tauber-Marshall 

[native-toolchain-CR] Bumped Kudu version to c6beta-impala-toolchain-tag1

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has abandoned this change. ( 
http://gerrit.cloudera.org:8080/9119 )

Change subject: Bumped Kudu version to c6beta-impala-toolchain-tag1
..


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ida50e3e9a9189e69013bc890401c9865c4682990
Gerrit-Change-Number: 9119
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Zoltan Borok-Nagy (Code Review)]

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 15:

Sorry, forgot to run the tests after changing HAVING's behavior.

It should pass now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 15
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 24 Jan 2018 16:48:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Zoltan Borok-Nagy (Code Review)]

Hello Taras Bobrovytsky, Tim Armstrong, Alex Behm, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5191: Standardize column alias behavior
..

IMPALA-5191: Standardize column alias behavior

We should not perform alias substitution in the
subexpressions of GROUP BY, HAVING, and ORDER BY
to be more standard conformant.

=== Allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY x;

SELECT NOT bool_col AS nb
FROM functional.alltypes
GROUP BY nb
HAVING nb;

=== Not allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x / 2;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY -x;

SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x
HAVING x > 3;

Some extra checks were added to AnalyzeExprsTest.java.

I had to update other tests to make them pass
since the new behavior is more restrictive.

I added alias.test to the end-to-end tests.

Cherry-picks: not for 2.x.

Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
---
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
A testdata/workloads/functional-query/queries/QueryTest/alias.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
M 
testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
M testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
M tests/query_test/test_queries.py
16 files changed, 252 insertions(+), 60 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 15
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners

[Pranay Singh (Code Review)]

Hello anujphadke, Tim Armstrong,

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

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

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

Change subject: IMPALA-3833: Fix invalid data handling in Sequence and RCFile 
scanners
..

IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners

Enabled the fuzz test for Sequence and RCFiles and added new checks to return
when failure is encountered while reading an RC file or sequence file.

Testing:
  a) Ran the fuzz test on a private build without failures/crash.
  b) Ran end to end test, back end tests with new changes.

Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
---
M be/src/exec/hdfs-rcfile-scanner.cc
M be/src/exec/hdfs-rcfile-scanner.h
M be/src/exec/hdfs-sequence-scanner.cc
M be/src/exec/hdfs-sequence-scanner.h
M be/src/exec/read-write-util-test.cc
M be/src/exec/read-write-util.h
M be/src/exec/scanner-context.inline.h
M be/src/util/decompress.cc
M tests/query_test/test_scanners_fuzz.py
9 files changed, 221 insertions(+), 61 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 8936
Gerrit-PatchSet: 5
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke