date:20171006

Hello Michael Ho, Tim Armstrong,

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

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

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

Change subject: IMPALA-4236: Codegen CopyRows() for select nodes
..

IMPALA-4236: Codegen CopyRows() for select nodes

Testing:
Added test case to verify that CopyRows in select node is successfully
codegened.

Performance:
Queries used (num_nodes set to 1):
500 Predicates: select * from (select * from tpch_parquet.lineitem
limit 6001215) t1 where l_partkey > 10 and l_extendedprice > 1 and
l_linenumber > 1 and l_comment >'foo0'  and l_comment >'foo500'
order by l_orderkey limit 10;

1 Predicate: select * from (select * from tpch_parquet.lineitem
limit 6001215) t1 where l_partkey > 10 order by l_orderkey limit 10;

+--+-+
|  |  500 Predicates  |   1 Predicate|
|  ++-++-+
|  |   After|   Before|   After|   Before|
+--++-++-+
| Select Node  | 12s385ms   | 1m1s| 234ms  | 797ms   |
| Codegen time | 2s619ms| 1s962ms | 200ms  | 181ms   |
+--++-++-+

Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/exec/CMakeLists.txt
A be/src/exec/select-node-ir.cc
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M tests/query_test/test_codegen.py
7 files changed, 110 insertions(+), 36 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e
Gerrit-Change-Number: 8196
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4236: Codegen CopyRows() for select nodes

Hello Michael Ho, Tim Armstrong,

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

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

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

Change subject: IMPALA-4236: Codegen CopyRows() for select nodes
..

IMPALA-4236: Codegen CopyRows() for select nodes

Testing:
Added test case to verify that CopyRows in select node is successfully
codegened.

Performance:
Queries used (num_nodes set to 1):
500 Predicates: select * from (select * from tpch_parquet.lineitem
limit 6001215) t1 where l_partkey > 10 and l_extendedprice > 1 and
l_linenumber > 1 and l_comment >'foo0'  and l_comment >'foo500'
order by l_orderkey limit 10;

1 Predicate: select * from (select * from tpch_parquet.lineitem
limit 6001215) t1 where l_partkey > 10 order by l_orderkey limit 10;

+--+-+
|  |  500 Predicates  |   1 Predicate|
|  ++-++-+
|  |   After|   Before|   After|   Before|
+--++-++-+
| Select Node  | 12s385ms   | 1m1s| 234ms  | 797ms   |
| Codegen time | 2s619ms| 1s962ms | 200ms  | 181ms   |
+--++-++-+

Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/exec/CMakeLists.txt
A be/src/exec/select-node-ir.cc
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M tests/query_test/test_codegen.py
7 files changed, 111 insertions(+), 36 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e
Gerrit-Change-Number: 8196
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4236: Codegen CopyRows() for select nodes

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8196 )

Change subject: IMPALA-4236: Codegen CopyRows() for select nodes
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8196/4/be/src/exec/select-node.cc
File be/src/exec/select-node.cc:

http://gerrit.cloudera.org:8080/#/c/8196/4/be/src/exec/select-node.cc@113
PS4, Line 113: copy_rows_result;
> I think it would still be better to have a name that makes the below contro
removed. see below comment.


http://gerrit.cloudera.org:8080/#/c/8196/4/be/src/exec/select-node.cc@120
PS4, Line 120: if (copy_rows_result) {
> I think there's an equivalent but simpler way of expressing the control flo
You are absolutely right. I have refactored the code which now also make the 
bool returned by CopyRows() redundant. Therefore I've changed CopyRows()'s 
return type to void.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0d496d004418468e16b6f564f90f45ebbf87c1e
Gerrit-Change-Number: 8196
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:53:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..

IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

In particular, the new query options:

BUFFER_POOL_LIMIT
MAX_ROW_SIZE
MIN_SPILLABLE_BUFFER_SIZE
DEFAULT_SPILLABLE_BUFFER_SIZE

Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Reviewed-on: http://gerrit.cloudera.org:8080/8003
Tested-by: Impala Public Jenkins
Reviewed-by: John Russell 
---
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
M docs/shared/impala_common.xml
A docs/topics/impala_buffer_pool_limit.xml
A docs/topics/impala_default_spillable_buffer_size.xml
A docs/topics/impala_max_row_size.xml
A docs/topics/impala_min_spillable_buffer_size.xml
M docs/topics/impala_scalability.xml
8 files changed, 619 insertions(+), 8 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 8
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/167/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:15:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7: Code-Review+2

Maybe I have to carry over Tim's +2 since I did another patch set after that.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:15:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5940: Avoid log spew by using Status::Expected.

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

Change subject: IMPALA-5940: Avoid log spew by using Status::Expected.
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
Gerrit-Change-Number: 8100
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:04:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5940: Avoid log spew by using Status::Expected.

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

Change subject: IMPALA-5940: Avoid log spew by using Status::Expected.
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8100/3/be/src/runtime/client-cache.h@246
PS3, Line 246: return 
Status::Expected(ErrorMsg(TErrorCode::RPC_RECV_TIMEOUT, err));
> I followed Michael's foot steps and looked at the ~17 callers of DoRpc. Far
works for me.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
Gerrit-Change-Number: 8100
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:03:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7:

Verification job appeared to succeed, but didn't find the commit ready to go in 
push_to_asf.py and it wasn't in master. Perhaps it got messed up because I was 
running 2 verification jobs simultaneously. Trying again.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 07 Oct 2017 00:00:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/166/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:58:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

2017-10-06 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7999 )

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1244
PS2, Line 1244: 2 GB, a serious error can occur. If only a limited 
number of partitions are actively being
> Done. "Serious error" was my compromise I always used for MySQL, where the
Sorry I just realized the ask here. Yea we do need some numbers like 
#partitions #files and #blocks to come up with an estimate of a particular 
table's contribution to the memory footprint.  Roughly something of the order of

#partitions * 2kB + #files * 750B + # file_blocks * 300B

The problem is that there is no easy way a user can get these numbers for a 
given table.

Similarly for incremental stats we can use the HMS dump to get an estimate (or 
directly connect to the HMS db)

select sum(length(PARTITION_PARAMS.PARAM_KEY) + 
length(PARTITION_PARAMS.PARAM_VALUE)), PARTITIONS.TBL_ID from PARTITIONS, 
PARTITION_PARAMS where PARTITIONS.PART_ID = PARTITION_PARAMS.PART_ID and 
PARTITION_PARAMS.PARAM_KEY LIKE 'impala_intermediate%' group by 
PARTITIONS.TBL_ID;

But in most serious setups, end users can't do this. IMPALA-4870 aims to expose 
most of this data (WIP). May be we can revisit this doc once it is done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:46:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:33:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..

[DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Explain how doing COMPUTE INCREMENTAL STATS for the first time
starts over and discards any previous stats from COMPUTE STATS.

As a consequence, moved some wording and examples into
impala_common.xml so that content could be used in
multiple places. Also made a new subtopic on the "Partitioning"
page because I saw COMPUTE INCREMENTAL STATS wasn't mentioned
there.

Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Reviewed-on: http://gerrit.cloudera.org:8080/7999
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M docs/shared/impala_common.xml
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_partitioning.xml
M docs/topics/impala_perf_stats.xml
4 files changed, 172 insertions(+), 110 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 6
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-5789: Add always false flag in bloom filter

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

Change subject: IMPALA-5789: Add always_false flag in bloom filter
..


Patch Set 2:

> Patch Set 1:
>
> (2 comments)
>
> Will wait for the new PS but had a couple of high-level bits of feedback 
> first. It may be worth looking at Thomas' patches if you haven't already to 
> make sure your changes are compatible.

This is ready for review. Thomas's min-max filter hasn't been merged and 
https://gerrit.cloudera.org/c/8148/ does not conflict with this patch.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d
Gerrit-Change-Number: 8170
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:33:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5789: Add always false flag in bloom filter

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5789: Add always_false flag in bloom filter
..

IMPALA-5789: Add always_false flag in bloom filter

This patch adds an always_false flag in bloom filters. The flag is set
if nothing has been inserted into the bloom filter. HdfsScanner uses
this flag to early terminate the scan at file and split granularities.

Testing: A test case is added checking that an always-false runtime
filter can filter out files and splits.

Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d
---
M be/src/common/global-flags.cc
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/base-sequence-scanner.h
M be/src/exec/filter-context.cc
M be/src/exec/filter-context.h
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scanner.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/runtime-filter-ir.cc
M be/src/runtime/runtime-filter.h
M be/src/runtime/runtime-filter.inline.h
M be/src/util/bloom-filter-ir.cc
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M common/thrift/ImpalaInternalService.thrift
A tests/custom_cluster/test_always_false_filter.py
21 files changed, 231 insertions(+), 196 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d
Gerrit-Change-Number: 8170
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:30:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5940: Avoid log spew by using Status::Expected.

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

Change subject: IMPALA-5940: Avoid log spew by using Status::Expected.
..


Patch Set 6:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8100/3/be/src/runtime/client-cache.h@246
PS3, Line 246:
> Maybe we should include the RPC name in the log message and that will disam
I followed Michael's foot steps and looked at the ~17 callers of DoRpc. Far 
from all of them log.

I'm now proposing we do this patch without the DoRpc() changes. From a 
performance standpoint, if you're timing out on RPCs, the backtrace is the 
least of your problems, and, yet, I think knowing which RPC is timing out is 
very helpful. I wasn't able to see a quick way to get the name of the RPC 
without discouraged C++ shenanigans. I checked that the Thrift Exception 
doesn't have that, nor do the Thrift structs seem to have self-identifying 
information.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
Gerrit-Change-Number: 8100
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:28:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5940: Avoid log spew by using Status::Expected.

Hello Michael Ho, Sailesh Mukil, Alex Behm, Mostafa Mokhtar, Dan Hecht,

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

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

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

Change subject: IMPALA-5940: Avoid log spew by using Status::Expected.
..

IMPALA-5940: Avoid log spew by using Status::Expected.

In IMPALA-5926, we fixed a case where closing the session triggered a
stack trace in the logs which impacted performance for short-running
queries. Looking at log files from several active clusters, I identified a few
other cases where we could clean up log spew with the same (trivial)
approach.

The following messages will no longer log:

  "Failed to codegen MaterializeTuple() due to unsupported type: $0"
  "Not implemented for this format."
  "ScalarFnCall Codegen not supported for CHAR"
  "Could not codegen CodegenMaterializeExprs: "
  "Could not codegen TupleRowComparator::Compare(): $0"

These codegen related messages were happening at every execution node, and were
therefore very common.

In addition, the following messages, which were very frequent in the logs, now
will log, but without the back trace:

  "Query Id ... not found."

To do this, I changed them from logging implicitly via Status(...)
to logging explicitly.

I have not fixed this message:
  "... Client ... timed-out during recv call."
It's in DoRpc(), and not all callers of DoRpc() have independent
logging. For now, we'll trade a little bit of log spam for clearer
debugging.

The snippet I used to identify these was:

  find . -type f -name '*IMPALAD*.gz' | xargs gzcat  | awk '/^I/ { if(x) { 
print x; } x = "" } /status.cc/ { x=" "; } { if(x) { x=x  $0 } }'  | sed -e 
's/0x[0-9a-fx]* //g' | sed -e 's/[0-9a-f]\{16\}:[0-9a-f]*/QUERYID/g' |  tr -s 
'\t' ' ' | tr '[0-9]' 'N' | sort | uniq -c  | sort -n | tee output.txt

I also analyzed some logs using SQL, against a pre-processed logs table:

  with v as (
select regexp_replace(
regexp_replace(
  translate(substr(message, 42), "\n\t", "  "),
  "[a-zA-Z0-9.-]*[.][a-zA-Z0-9-]*:[0-9]*",
  ""),
"@.*$", "@@@...") as m
from logs_table where `class`="status.cc")
  select m, count(*) from v group by 1 order by 2 desc limit 100

Testing:
* Automated tests.
* Manual testing for one of the new back-trace-suppressed paths:

  $ impala-python shell/gen-py/ImpalaService/ImpalaService-remote -h 
localhost:21000 GetRuntimeProfile 'beeswaxd.ttypes.QueryHandle()'
  Traceback (most recent call last):
File "shell/gen-py/ImpalaService/ImpalaService-remote", line 106, in 

  pp.pprint(client.GetRuntimeProfile(eval(args[0]),))
File "/home/philip/src/impala/shell/gen-py/ImpalaService/ImpalaService.py", 
line 161, in GetRuntimeProfile
  return self.recv_GetRuntimeProfile()
File "/home/philip/src/impala/shell/gen-py/ImpalaService/ImpalaService.py", 
line 184, in recv_GetRuntimeProfile
  raise result.error
  beeswaxd.ttypes.BeeswaxException: 
BeeswaxException(handle=QueryHandle(log_context='', id=''), log_context='', 
SQLState='HY000', _message='GetRuntimeProfile error: Query id 0:0 not 
found.\n', errorCode=0)

  $ grep 'Query id' logs/cluster/impalad.INFO | tail -n 1
  I0926 20:29:09.44  6787 impala-server.cc:642] Query id 0:0 not found.

Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
---
M be/src/exec/hdfs-avro-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exprs/scalar-fn-call.cc
M be/src/runtime/tuple.cc
M be/src/service/impala-server.cc
M be/src/util/tuple-row-compare.cc
6 files changed, 16 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38088482377a1c3e794a9c8178ef83f29957a330
Gerrit-Change-Number: 8100
Gerrit-PatchSet: 6
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/165/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:25:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker errors

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

Change subject: IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker 
errors
..


Patch Set 1:

(5 comments)

I'm excited by anything that makes LLVM debugging easier!

http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.h
File be/src/codegen/llvm-codegen.h:

http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.h@783
PS1, Line 783:   class DiagnosticState {
Based on http://llvm.org/doxygen/DiagnosticHandler_8h_source.html, it looks 
like they recommend us subclassing their class to implement this, and that 
there are different "remarks" that we can get.

It's tricky, but if they've got different warnings, I sure as heck would want 
to see them during development.


http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.cc@186
PS1, Line 186:   >diagnostic_state_, true);
Could you comment on what this third argument does?

The llvm header says "expects enabled diagnostics", which I don't understand.


http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.cc@304
PS1, Line 304:   if (error || diagnostic_state_.EncounteredError()) {
So do the diagnostic messages get printed anywhere?


http://gerrit.cloudera.org:8080/#/c/8233/1/be/src/codegen/llvm-codegen.cc@1622
PS1, Line 1622: info.print(diagnostic_printer);
Does this make it to our logs?


http://gerrit.cloudera.org:8080/#/c/8233/1/tests/query_test/test_codegen.py
File tests/query_test/test_codegen.py:

http://gerrit.cloudera.org:8080/#/c/8233/1/tests/query_test/test_codegen.py@45
PS1, Line 45:   def test_codegen_diagnostic_handler(self, vector):
You could probably test this in be/src/codegen/llvm-codegen-test.cc with 
considerably less fanfare. I think the end-to-end test that we don't fail on a 
custom UDF is lovely, but a more targeted test makes sense too.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I907ff1f8975c02d422b4a669afbfc2e536ddf1ff
Gerrit-Change-Number: 8233
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:22:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2636: HS2 GetTables() returns TABLE TYPE as TABLE for VIEW

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for 
VIEW
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90616388e6181cf342b3de389af940214ed46428
Gerrit-Change-Number: 7353
Gerrit-PatchSet: 4
Gerrit-Owner: sandeep akinapelli 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: sandeep akinapelli 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:08:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/164/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:25:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

Hello Tim Armstrong, Mostafa Mokhtar, Dan Hecht,

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

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

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer 
pool
..

IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

In particular, the new query options:

BUFFER_POOL_LIMIT
MAX_ROW_SIZE
MIN_SPILLABLE_BUFFER_SIZE
DEFAULT_SPILLABLE_BUFFER_SIZE

Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
---
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
M docs/shared/impala_common.xml
A docs/topics/impala_buffer_pool_limit.xml
A docs/topics/impala_default_spillable_buffer_size.xml
A docs/topics/impala_max_row_size.xml
A docs/topics/impala_min_spillable_buffer_size.xml
M docs/topics/impala_scalability.xml
8 files changed, 622 insertions(+), 8 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-3200: [DOCS] Document user-facing aspects of new buffer pool

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

Change subject: IMPALA-3200: [DOCS] Document user-facing aspects of new buffer
pool
..

Patch Set 7:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_buffer_pool_limit.xml
File docs/topics/impala_buffer_pool_limit.xml:

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_buffer_pool_limit.xml@53
PS5, Line 53:
:
:
> This area of behaviour in the code is a bit of a mess and subject to change
Done

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml
File docs/topics/impala_default_spillable_buffer_size.xml:

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml@72
PS5, Line 72: spill-to-disk operations. Reducing this value may reduce
memory consumption.
> The last sentence I think creates a lot of potential for confusion. The con
Done

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_default_spillable_buffer_size.xml@73
PS5, Line 73:
> Impala daemons?
Done

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml
File docs/topics/impala_max_row_size.xml:

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml@40
PS5, Line 40: , but that is not guaranteed.) Applies when
: constructing intermediate or final rows in the result set.
This setting prevents
: out-of-control memory use when accessing columns containing
huge strings.
:
> This summary is wrong (the behaviour is subtle). The query option has zero
Done

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_max_row_size.xml@69
PS5, Line 69: his setting.
> This is correct but I'm concerned some users might take it the wrong way. T
OK, will do. But is it not reasonable to worry a little if some rows are > 512
KB? I would imagine that some subtle change (adding or removing a host, HDFS
rebalancing, changing the WHERE condition, updating some rows in a Kudu table,
changing an impalad config setting) could cause big-row queries to fail where
previously they succeeded.

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_min_spillable_buffer_size.xml
File docs/topics/impala_min_spillable_buffer_size.xml:

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_min_spillable_buffer_size.xml@49
PS5, Line 49:
> Same comments as default_spillable_buffer_size.
OK. I added a summary in the first paragraph, which like the DEFAULT_ topic was
originally blank. I copied the wording about the tradeoff with a too-high
value... but could a user actually lower this setting below 64KB? I got the
impression maybe 64KB was already at the low end of the allowable range.

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml
File docs/topics/impala_scalability.xml:

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@570
PS5, Line 570:On each host that participates in the query, each such
operator in a query requires memory
> I think this background is helpful for users to understand the implications
OK, used your suggested wording.

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@572
PS5, Line 572:up front for each operator that supports spill-to-disk
that is sufficient to execute the
> Could we keep this around but make clear that it describes the behaviour in
Let's revisit that later. Trying to tease out what is needed in a "Previously,"
section could be problematic so close to the deadline.

http://gerrit.cloudera.org:8080/#/c/8003/5/docs/topics/impala_scalability.xml@627
PS5, Line 627:
> Should we mention the old names parenthetically (it was ScratchBytesWritten
Done

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49323f8ffbff3e195058e88762eedbb1fcb1bc0e
Gerrit-Change-Number: 8003
Gerrit-PatchSet: 7
Gerrit-Owner: John Russell
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: John Russell
Gerrit-Reviewer: Mostafa Mokhtar
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:23:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5789: Add always false flag in bloom filter

Tianyi Wang has restored this change. ( http://gerrit.cloudera.org:8080/8170 )

Change subject: IMPALA-5789: Add always_false flag in bloom filter
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: restore
Gerrit-Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d
Gerrit-Change-Number: 8170
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-2636: HS2 GetTables() returns TABLE TYPE as TABLE for VIEW

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

Change subject: IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for 
VIEW
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90616388e6181cf342b3de389af940214ed46428
Gerrit-Change-Number: 7353
Gerrit-PatchSet: 4
Gerrit-Owner: sandeep akinapelli 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: sandeep akinapelli 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:07:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2636: HS2 GetTables() returns TABLE TYPE as TABLE for VIEW

2017-10-06 Thread sandeep akinapelli (Code Review)

sandeep akinapelli has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7353 )

Change subject: IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for 
VIEW
..


Patch Set 4:

(4 comments)

addressed more comments.

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

http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/main/java/org/apache/impala/service/MetadataOp.java@496
PS3, Line 496: tableType = tableType.toUpperCase();
> I think the toUpperCase() here should stay.
Done


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/main/java/org/apache/impala/service/MetadataOp.java@497
PS3, Line 497: upperCaseTableTypes.add(tableType);
> You can use equals() if you leave the toUpperCase() above.
Done


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java
File fe/src/test/java/org/apache/impala/service/FrontendTest.java:

http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java@181
PS3, Line 181: TMetadataOpRequest req = new TMetadataOpRequest();
> space after "default"
not a relevant comment. removed


http://gerrit.cloudera.org:8080/#/c/7353/3/fe/src/test/java/org/apache/impala/service/FrontendTest.java@209
PS3, Line 209: TResultSet resp = execMetadataOp(req);
> Does this test succeed when run on its own? For these views to be returned
right. this could be flakey. added analyze statements to make sure these are 
loaded.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90616388e6181cf342b3de389af940214ed46428
Gerrit-Change-Number: 7353
Gerrit-PatchSet: 4
Gerrit-Owner: sandeep akinapelli 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: sandeep akinapelli 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:06:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 5: Code-Review+2

I'm still working with Bharath on getting diagnostics for the table sizes, but 
this patch looks good.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 23:07:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2636: HS2 GetTables() returns TABLE TYPE as TABLE for VIEW

2017-10-06 Thread sandeep akinapelli (Code Review)

Hello Matthew Jacobs, Alex Behm, Vuk Ercegovac,

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

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

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

Change subject: IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for 
VIEW
..

IMPALA-2636: HS2 GetTables() returns TABLE_TYPE as TABLE for VIEW

Added code to read the table type from metastore table but defaults to
"TABLE" if the metastore table is not loaded.
After the change, GetTabletypes also returns "VIEW" apart from "TABLE"
Changed unit and jdbc testcases for GetTableTypes.
Added new Frontend test for reading views.

Change-Id: I90616388e6181cf342b3de389af940214ed46428
---
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/test/java/org/apache/impala/service/FrontendTest.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
3 files changed, 142 insertions(+), 23 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I90616388e6181cf342b3de389af940214ed46428
Gerrit-Change-Number: 7353
Gerrit-PatchSet: 4
Gerrit-Owner: sandeep akinapelli 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: sandeep akinapelli

[Impala-ASF-CR] IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker errors

Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8233


Change subject: IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker 
errors
..

IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker errors

Currently if llvm encounters an error it calls exit() and this kills
the impalad process. This patch adds a diagnostic handler that llvm
can use to pass the error up to impala instead of crashing it.

Testing:
Added a test which induces an error in llvm and makes sure that its
passed up to impala code and handled correctly.

Change-Id: I907ff1f8975c02d422b4a669afbfc2e536ddf1ff
---
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M tests/query_test/test_codegen.py
3 files changed, 67 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I907ff1f8975c02d422b4a669afbfc2e536ddf1ff
Gerrit-Change-Number: 8233
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 22:29:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..

IMPALA-5529: [DOCS] New trunc() signatures

Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Reviewed-on: http://gerrit.cloudera.org:8080/8189
Reviewed-by: Greg Rahn 
Reviewed-by: Thomas Tauber-Marshall 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_datetime_functions.xml
M docs/topics/impala_math_functions.xml
2 files changed, 170 insertions(+), 17 deletions(-)

Approvals:
  Greg Rahn: Looks good to me, but someone else must approve
  Thomas Tauber-Marshall: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/163/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 22:24:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1227
PS4, Line 1227: COMPUTE INCREMENTAL STATS during the 
lifetime of a table,
> (or vice versa)
Done


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1243
PS4, Line 1243: be cached on every impalad host that 
is eligible to be a coordinator.
> as it must be cached on the catalogd and on every ...
Done


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1244
PS4, Line 1244: If this metadata for a table exceeds 2 GB, you might 
experience service downtime.
> It's worse than that. If the aggregate metadata of *all* tables combined ge
Done


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_partitioning.xml@612
PS4, Line 612: as new partitions are added, Impala includes a variation 
of this statement that is intended for use with
> How about:
Done


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_perf_stats.xml
File docs/topics/impala_perf_stats.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_perf_stats.xml@361
PS4, Line 361:   COMPUTE STATS statement might take 
hours, or even days. For such tables, use
> Sorry, I disagree with the "For such tables, use COMPUTE INCREMENTAL STATS"
OK, since this bit is just a pointer to the real writeup elsewhere, why don't I 
delete the  entirely rather than trying to find some softer wording.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 22:22:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Hello Greg Rahn, Silvius Rus, Alex Behm, Mostafa Mokhtar, Vuk Ercegovac,

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

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

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..

[DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Explain how doing COMPUTE INCREMENTAL STATS for the first time
starts over and discards any previous stats from COMPUTE STATS.

As a consequence, moved some wording and examples into
impala_common.xml so that content could be used in
multiple places. Also made a new subtopic on the "Partitioning"
page because I saw COMPUTE INCREMENTAL STATS wasn't mentioned
there.

Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
---
M docs/shared/impala_common.xml
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_partitioning.xml
M docs/topics/impala_perf_stats.xml
4 files changed, 172 insertions(+), 110 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] Allow the SASL protocol service name to be configurable

2017-10-06 Thread Sailesh Mukil (Code Review)

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

Change subject: Allow the SASL protocol service name to be configurable
..


Patch Set 1:

This patch allows Impala to pass in the SASL service name down into the KRPC 
library, so that we can avoid code divergence.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e30fe4461893b67527333259579e2304b19af1e
Gerrit-Change-Number: 8230
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 06 Oct 2017 22:09:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] Allow the SASL protocol service name to be configurable

2017-10-06 Thread Sailesh Mukil (Code Review)

Hello Dan Burkert,

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

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

to review the following change.


Change subject: Allow the SASL protocol service name to be configurable
..

Allow the SASL protocol service name to be configurable

Previously the SASL service name was always set to a constant "kudu"
which was tracked by kSaslProtoName in rpc/constants.h.

However, for applications that use the KRPC library that would prefer
to do their own SASL initialization, they would requre to set their own
SASL service name to be passed into sasl_server_new()/sasl_client_new().

This patch allows for this configuration by adding a configurable
parameter to the MessengerBuilder which is ultimately passed down to the
negotiation layer.

Change-Id: I9e30fe4461893b67527333259579e2304b19af1e
Reviewed-on: http://gerrit.cloudera.org:8080/8218
Reviewed-by: Dan Burkert 
Tested-by: Dan Burkert 
---
M be/src/kudu/rpc/client_negotiation.cc
M be/src/kudu/rpc/client_negotiation.h
M be/src/kudu/rpc/constants.cc
M be/src/kudu/rpc/constants.h
M be/src/kudu/rpc/messenger.cc
M be/src/kudu/rpc/messenger.h
M be/src/kudu/rpc/negotiation-test.cc
M be/src/kudu/rpc/negotiation.cc
M be/src/kudu/rpc/server_negotiation.cc
M be/src/kudu/rpc/server_negotiation.h
10 files changed, 59 insertions(+), 28 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e30fe4461893b67527333259579e2304b19af1e
Gerrit-Change-Number: 8230
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Burkert

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

2017-10-06 Thread Michael Ho (Code Review)

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

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


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8023/2/be/src/runtime/krpc-data-stream-recvr.cc@141
PS2, Line 141:   while (true) {
 : // wait until something shows up or we know we're done
 : while (!is_cancelled_ && batch_queue_.empty() && 
blocked_senders_.empty()
 : && num_remaining_senders_ > 0) {
 :   VLOG_ROW << "wait arrival fragment_instance_id=" << 
recvr_->fragment_instance_id()
 :<< " node=" << recvr_->dest_node_id();
 :   // Don't count time spent waiting on the sender as active 
time.
 :   CANCEL_SAFE_SCOPED_TIMER(recvr_->data_arrival_timer_, 
_cancelled_);
 :   CANCEL_SAFE_SCOPED_TIMER(recvr_->inactive_timer_, 
_cancelled_);
 :   CANCEL_SAFE_SCOPED_TIMER(
 :   received_first_batch_ ? nullptr : 
recvr_->first_batch_wait_total_timer_,
 :   _cancelled_);
 :   data_arrival_cv_.wait(l);
 : }
 :
 : if (is_cancelled_) return Status::CANCELLED;
 :
 : if (blocked_senders_.empty() && batch_queue_.empty()) {
 :   DCHECK_EQ(num_remaining_senders_, 0);
 :   return Status::OK();
 : }
 :
 : received_first_batch_ = true;
 :
 : // Either we'll consume a row batch from batch_queue_, or 
it's empty. In either case,
 : // take a blocked sender and retry delivering their batch. 
There is a window between
 : // which a deferred batch is dequeued from blocked_senders_ 
queue and when it's
 : // inserted into batch_queue_. However, a receiver won't 
respond to the sender until
 : // the deferred row batch has been inserted. The sender will 
wait for all in-flight
 : // RPCs to complete before sending EOS RPC so 
num_remaining_senders_ should be > 0.
 : if (!blocked_senders_.empty()) {
 :   recvr_->mgr_->EnqueueRowBatch(
 :   {recvr_->fragment_instance_id(), 
move(blocked_senders_.front())});
 :   blocked_senders_.pop();
 : }
 :
 : if (!batch_queue_.empty()) {
 :   RowBatch* result = batch_queue_.front().second;
 :   
recvr_->num_buffered_bytes_.Add(-batch_queue_.front().first);
 :   VLOG_ROW << "fetched #rows=" << result->num_rows();
 :   current_batch_.reset(result);
 :   *next_batch = current_batch_.get();
 :   batch_queue_.pop_front();
 :   return Status::OK();
 : }
> This loop may lead to live lock in the rare case in which blocked_senders_
Actually, mis-read the thing in the heat of debugging. If both queues are 
empty, we may return early in line 160 above if num_remaining_senders == 0. So, 
we shouldn't spin forever. Otherwise, the thread should sleep and wait in line 
153. This loop tends to have the unfortunate behavior of popping all entries 
off blocked_senders_ first before dropping the lock and sleeping on line 153.

Although there is a window in which both queues are empty when a row batch is 
deserialized and moved from blocked_senders_ to batch_queue_, it should be 
impossible for num_remaining_senders_ to reach 0 in that window. The reason is 
that the sender of that row batch will not be responded to until after the row 
batch has been inserted into batch_queue_ (after it has been popped from 
blocked_senders_). In which case, batch_queue_ will become non-empty first 
before the remote sender gets a reply.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0b8c1e50678da66ab1547d16530f88b323ed8c1
Gerrit-Change-Number: 8023
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 06 Oct 2017 22:03:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Thomas Tauber-Marshall (Code Review)

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:59:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Abandoned

Need to sync up with Tim before moving forward.
--
To view, visit http://gerrit.cloudera.org:8080/8226
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Patch Set 1:

> Patch Set 1:
>
> > (1 comment)
>
> Correct, it will unpin/pin multiple times. But if the memory is available the 
> buffers will stay in memory even though it was unpinned (you'll want to take 
> a look through the buffer pool code starting with the header comments). If 
> the memory is not available, then we will read from disk but compare that to 
> today where we'd run out of memory and fail. It does mean, however, that rows 
> are "unflattened" multiple times.
>
> It would be good to sync up with Tim about this JIRA to ask his intention 
> when he returns on Monday.  I believe the JIRA was filed a while back before 
> we did a big rework of these layers (I think the JIRA still makes sense, but 
> good to sync up).

Thanks. I will abandon this patch for now.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:56:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Patch Set 1:

> (1 comment)

Correct, it will unpin/pin multiple times. But if the memory is available the 
buffers will stay in memory even though it was unpinned (you'll want to take a 
look through the buffer pool code starting with the header comments). If the 
memory is not available, then we will read from disk but compare that to today 
where we'd run out of memory and fail. It does mean, however, that rows are 
"unflattened" multiple times.

It would be good to sync up with Tim about this JIRA to ask his intention when 
he returns on Monday.  I believe the JIRA was filed a while back before we did 
a big rework of these layers (I think the JIRA still makes sense, but good to 
sync up).


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:54:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1227
PS4, Line 1227: COMPUTE INCREMENTAL STATS during the 
lifetime of a table,
(or vice versa)


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1243
PS4, Line 1243: be cached on every impalad host that 
is eligible to be a coordinator.
as it must be cached on the catalogd and on every ...


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/shared/impala_common.xml@1244
PS4, Line 1244: If this metadata for a table exceeds 2 GB, you might 
experience service downtime.
It's worse than that. If the aggregate metadata of *all* tables combined gets 
to 2GB you may experience downtime.


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_partitioning.xml@612
PS4, Line 612: as new partitions are added, Impala includes a variation 
of this statement that is intended for use with
How about:

includes a variation of this statement that allows computing statistics on a 
per-partition basis such that stats can be incrementally updated when new 
partitions are added.


http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_perf_stats.xml
File docs/topics/impala_perf_stats.xml:

http://gerrit.cloudera.org:8080/#/c/7999/4/docs/topics/impala_perf_stats.xml@361
PS4, Line 361:   COMPUTE STATS statement might take 
hours, or even days. For such tables, use
Sorry, I disagree with the "For such tables, use COMPUTE INCREMENTAL STATS" 
part. I think we need to be very careful about recommending incremental stats. 
We can document what it does, but I think we should go out of out way to not 
explicitly recommend it for any reason.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:53:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Hello Greg Rahn, Silvius Rus, Alex Behm, Mostafa Mokhtar, Vuk Ercegovac,

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

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

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..

[DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Explain how doing COMPUTE INCREMENTAL STATS for the first time
starts over and discards any previous stats from COMPUTE STATS.

As a consequence, moved some wording and examples into
impala_common.xml so that content could be used in
multiple places. Also made a new subtopic on the "Partitioning"
page because I saw COMPUTE INCREMENTAL STATS wasn't mentioned
there.

Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
---
M docs/shared/impala_common.xml
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_partitioning.xml
M docs/topics/impala_perf_stats.xml
4 files changed, 180 insertions(+), 107 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(3 comments)

OK, I think this is about is far as we can go given the scope of the original 
request, to clarify that you use CS or CIS but not both. I got more details 
consulting with Alex but that would require something like a tutorial or deep 
dive to cover what's happening and the tradeoffs for each aspect.

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1233
PS2, Line 1233: When you run COMPUTE INCREMENTAL STATS 
on a table for the first time,
> I suggest some minor rephrasing to drive home the "don't switch mantra" a l
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1245
PS2, Line 1245: added or inserted into, you can run COMPUTE 
INCREMENTAL STATS for the active
> Sorry my phrasing might have been misleading. By "active" partitions I mean
OK, after consulting with Alex I'm paring this wording way back. Just too many 
ways that someone could do extra work that was counterproductive or didn't have 
the benefit that they assumed.


http://gerrit.cloudera.org:8080/#/c/7999/3/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/3/docs/topics/impala_partitioning.xml@623
PS3, Line 623: is a shortcut for partitioned tables that works on a
 : subset of partitions rather than the entire table.
> How about:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:26:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5425: Add test for validating input when setting query options

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

Change subject: IMPALA-5425: Add test for validating input when setting query 
options
..


Patch Set 14:

> Uploaded patch set 14.

Rebase


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I510e02bb0776673d8cbfc22b903831882c6908d7
Gerrit-Change-Number: 7805
Gerrit-PatchSet: 14
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 21:00:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5425: Add test for validating input when setting query options

Hello Lars Volker, Tim Armstrong, Alex Behm, Dan Hecht,

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

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

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

Change subject: IMPALA-5425: Add test for validating input when setting query 
options
..

IMPALA-5425: Add test for validating input when setting query options

This patch adds multiple query option validation testcases to
be/src/service/query-options-test.cc
The test cases include parsing edge cases, bondary values, special
cases for some options and some testcases moved from
testdata/workloads/functional-query/queries/QueryTest/set.test
This patch also fixes a bug generating wrong error message for
query option RUNTIME_FILTER_WAIT_TIME_MS.

Change-Id: I510e02bb0776673d8cbfc22b903831882c6908d7
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
3 files changed, 267 insertions(+), 128 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I510e02bb0776673d8cbfc22b903831882c6908d7
Gerrit-Change-Number: 7805
Gerrit-PatchSet: 14
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 20:05:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Lars Volker (Code Review)

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4:

I lack context here, but I'll ask Thomas to have another look. He was one of 
the reviewers of the original change.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:29:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4704: Disallow client connections to imapalad until catalog is received.

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

Change subject: IMPALA-4704: Disallow client connections to imapalad until 
catalog is received.
..


Patch Set 4:

Update on this one. Several e2e tests failed so I'm fixing them. Main issue 
that I've found so far is how various tests check that a cluster is ready.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6
Gerrit-Change-Number: 8202
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:14:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..

IMPALA-3504: [DOCS] Document utc_timestamp()

This function seems to be related to
unix_micros_to_utc_timestamp() and
utc_to_unix_micros() which are also not
currently documented. Did they all come in
at the same time or have the *unix_micros*
ones existed for a while?

Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Reviewed-on: http://gerrit.cloudera.org:8080/8190
Reviewed-by: Greg Rahn 
Reviewed-by: Lars Volker 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_datetime_functions.xml
1 file changed, 56 insertions(+), 0 deletions(-)

Approvals:
  Greg Rahn: 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/8190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Ivanfi

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:10:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/162/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:06:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8226/1//COMMIT_MSG@10
PS1, Line 10: which returns multiple batches instead of one.
> that will cause us to still have to pin the entire stream in memory.  Inste
The callers iterates through the build rows for every row in 
null_probe_rows/null_aware_probe_partition_. If we don't pin the stream we need 
to read the stream multi times, for every prob batch. Sure about that?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:03:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Lars Volker (Code Review)

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1: Code-Review+2

This looks good to me. Zoltan should also have a look at this. Unfortunately 
he's in CET so I suggest to move on with this and then fix any issues he finds 
in a subsequent commit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 19:02:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8226/1//COMMIT_MSG@10
PS1, Line 10: which returns multiple batches instead of one.
that will cause us to still have to pin the entire stream in memory.  Instead, 
I think the ask of the JIRA is the change the callers to use the GetNext() 
interface, so that they only need to keep a single batch in memory at a time, 
since I believe all callers are really just iterating through the stream anyway.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:47:32 +
Gerrit-HasComments: Yes

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

Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7938 )

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


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cea56cc6e7412d87f4c2e92399a2f91ea6af6c7
Gerrit-Change-Number: 7938
Gerrit-PatchSet: 5
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:42:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

2017-10-06 Thread Thomas Tauber-Marshall (Code Review)

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

Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/8226/1//COMMIT_MSG@14
PS1, Line 14:
Any testing?


http://gerrit.cloudera.org:8080/#/c/8226/1/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/8226/1/be/src/exec/partitioned-hash-join-node.cc@950
PS1, Line 950: break;
I think this changes the behavior here - this will only break out of the 
FOREACH_ROW loop, but what we want is to stop iterating over the entire set of 
build rows (that was previously one batch, but is now the same set of rows 
split into multiple batches).


http://gerrit.cloudera.org:8080/#/c/8226/1/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:

http://gerrit.cloudera.org:8080/#/c/8226/1/be/src/runtime/buffered-tuple-stream.h@339
PS1, Line 339: vector* batch
Can you move this to be after 'batch_size'? We generally keep out parameters as 
the trailing parameters for clarity.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:24:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

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

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4:

Tagging in Lars to the review in case he would like to do the +2 based on 
Greg's +1. (I consider Greg the best arbiter of what makes sense to cover in 
terms of the built-in functions.)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:21:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7999/3/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/3/docs/topics/impala_partitioning.xml@623
PS3, Line 623: is a shortcut for partitioned tables that works on a
 : subset of partitions rather than the entire table.
How about:

"that works only on the subset of partitions of a partitioned table that have 
changed."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 18:21:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(19 comments)

Almost finished with the comments. I'll touch base with Alex to get a little 
more clarification about which stats are safe to, or make sense to, DROP 
INCREMENTAL STATS for.

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1224
PS2, Line 1224: For a particular table, use either COMPUTE 
STATS or
> Yes!
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1228
PS2, Line 1228: DROP STATS and
  : DROP INCREMENTAL STATS)
> They are not required if you *exactly* what you are doing, but that does no
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1228
PS2, Line 1228: DROP STATS and
  : DROP INCREMENTAL STATS)
> are these drops required?
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1234
PS2, Line 1234: the statistics are computed again from scratch 
regardless of whether you previously ran
> regardless of whether the table has existing stats.
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1236
PS2, Line 1236: for scanning the entire table when switching from 
COMPUTE STATS to
> when running COMPUTE INCREMENTAL STATS for the first time on a given table.
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: be cached on every impalad host. If 
this metadata for a table exceeds
> more specifically, impalads that are also coordinators?
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: be cached on every impalad host. If 
this metadata for a table exceeds
> Yes
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1244
PS2, Line 1244: 2 GB, a serious error can occur. If only a limited 
number of partitions are actively being
> If the aggregate metadata of all tables exceeds 2 GB you may experience ser
Done. "Serious error" was my compromise I always used for MySQL, where the open 
source tradition leaned towards saying "crash" but the enterprise focus 
suggested something more euphemistic.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> Fine with me to expand this to add my earlier explanation of what the "incr
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> does that mean lack of stats has not affect on optimization or something el
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> fair pointer for me, but my comment is about whether this wording is clear
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> Please see my explanation on what "incremental" stats is in previous patch
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1248
PS2, Line 1248: optimizations such as partition pruning.
> such as partition pruning or join ordering.
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1248
PS2, Line 1248: optimizations such as partition pruning.
> Actually I would remove partition pruning because stats have nothing to do
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml@611
PS2, Line 611: frequently
> remove
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml@623
PS2, Line 623: is a shortcut
> I don't know what "shortcut" means here. I'd remove it.
I'm looking for a way to convey that it's faster to do COMPUTE INCREMENTAL 
STATS on a partitioned table than COMPUTE STATS. But the time savings only 
happens if you do C.I.S. multiple times, that is, because the table keeps 
getting new partitions.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml
File docs/topics/impala_perf_stats.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml@361
PS2, Line 361:   COMPUTE STATS statement might take 
hours, or even days. That situation is where you switch
> Rephrase to avoid "switch" since switching is bad
Done


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml@361
PS2, Line 361: That situation is where you switch
> I'd reword this part ("That situation is where ...").

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Hello Greg Rahn, Silvius Rus, Alex Behm, Mostafa Mokhtar, Vuk Ercegovac,

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

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

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..

[DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

Explain how doing COMPUTE INCREMENTAL STATS for the first time
starts over and discards any previous stats from COMPUTE STATS.

As a consequence, moved some wording and examples into
impala_common.xml so that content could be used in
multiple places. Also made a new subtopic on the "Partitioning"
page because I saw COMPUTE INCREMENTAL STATS wasn't mentioned
there.

Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
---
M docs/shared/impala_common.xml
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_partitioning.xml
M docs/topics/impala_perf_stats.xml
4 files changed, 184 insertions(+), 107 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac

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

2017-10-06 Thread Michael Ho (Code Review)

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

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


Patch Set 2:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/8023/2/be/src/runtime/krpc-data-stream-recvr.cc@141
PS2, Line 141:   while (true) {
 : // wait until something shows up or we know we're done
 : while (!is_cancelled_ && batch_queue_.empty() && 
blocked_senders_.empty()
 : && num_remaining_senders_ > 0) {
 :   VLOG_ROW << "wait arrival fragment_instance_id=" << 
recvr_->fragment_instance_id()
 :<< " node=" << recvr_->dest_node_id();
 :   // Don't count time spent waiting on the sender as active 
time.
 :   CANCEL_SAFE_SCOPED_TIMER(recvr_->data_arrival_timer_, 
_cancelled_);
 :   CANCEL_SAFE_SCOPED_TIMER(recvr_->inactive_timer_, 
_cancelled_);
 :   CANCEL_SAFE_SCOPED_TIMER(
 :   received_first_batch_ ? nullptr : 
recvr_->first_batch_wait_total_timer_,
 :   _cancelled_);
 :   data_arrival_cv_.wait(l);
 : }
 :
 : if (is_cancelled_) return Status::CANCELLED;
 :
 : if (blocked_senders_.empty() && batch_queue_.empty()) {
 :   DCHECK_EQ(num_remaining_senders_, 0);
 :   return Status::OK();
 : }
 :
 : received_first_batch_ = true;
 :
 : // Either we'll consume a row batch from batch_queue_, or 
it's empty. In either case,
 : // take a blocked sender and retry delivering their batch. 
There is a window between
 : // which a deferred batch is dequeued from blocked_senders_ 
queue and when it's
 : // inserted into batch_queue_. However, a receiver won't 
respond to the sender until
 : // the deferred row batch has been inserted. The sender will 
wait for all in-flight
 : // RPCs to complete before sending EOS RPC so 
num_remaining_senders_ should be > 0.
 : if (!blocked_senders_.empty()) {
 :   recvr_->mgr_->EnqueueRowBatch(
 :   {recvr_->fragment_instance_id(), 
move(blocked_senders_.front())});
 :   blocked_senders_.pop();
 : }
 :
 : if (!batch_queue_.empty()) {
 :   RowBatch* result = batch_queue_.front().second;
 :   
recvr_->num_buffered_bytes_.Add(-batch_queue_.front().first);
 :   VLOG_ROW << "fetched #rows=" << result->num_rows();
 :   current_batch_.reset(result);
 :   *next_batch = current_batch_.get();
 :   batch_queue_.pop_front();
 :   return Status::OK();
 : }
This loop may lead to live lock in the rare case in which blocked_senders_ and 
batch_queue_ are both empty in the window in which the deserialization threads 
are still working on inserting the row batches in blocked_senders_ into 
batch_queue_ after batch_queue_ has been exhausted. Spinning here forever will 
not drop the lock, causing the deserialization threads to wait forever to 
insert into batch_queue_.


http://gerrit.cloudera.org:8080/#/c/8023/2/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/8023/2/be/src/runtime/krpc-data-stream-sender.cc@485
PS2, Line 485: // Sleep for sometime before retrying.
 : if (RpcMgr::IsServerTooBusy(rpc_controller_)) {
 :   SleepForMs(FLAGS_rpc_retry_interval_ms);
 :   continue;
 : }
This is broken if the RPC was rejected for 
FLAGS_backend_client_connection_num_retries number of times in a row. In which 
case, we will break out of the loop and return Status::OK(). This can lead to 
remote receiver hanging forever as it still thinks there are still active 
senders.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0b8c1e50678da66ab1547d16530f88b323ed8c1
Gerrit-Change-Number: 8023
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:58:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> fair pointer for me, but my comment is about whether this wording is clear
Fine with me to expand this to add my earlier explanation of what the 
"incremental" part of the stats actually is (to explain why dropping the 
"incremental" portion is fine).


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1248
PS2, Line 1248: optimizations such as partition pruning.
> such as partition pruning or join ordering.
Actually I would remove partition pruning because stats have nothing to do with 
that (and we don't want to imply that they do)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:46:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1:

Lars, should be ready for +2 from any committer.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:40:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu

2017-10-06 Thread Thomas Tauber-Marshall (Code Review)

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

Change subject: IMPALA-4252: Min-max runtime filters for Kudu
..


Patch Set 6:

(32 comments)

http://gerrit.cloudera.org:8080/#/c/7793/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/7793/5//COMMIT_MSG@9
PS5, Line 9: This patch implements min-max filters for runtime filters. Each
> I had a general terminology question that popped up as I read through. Does
My intention (which I probably wasn't super consistent about, I'll try to clean 
it up) was that a "runtime filter" corresponds to a single filter id and might 
contain and bloom and/or min-max (eventually and/or in-list) filters.


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.h
File be/src/exec/filter-context.h:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.h@80
PS5, Line 80: /// FilterContext contains all metadata for a single runtime 
filter, and allows the filter
> This seems to be an example of a place where "runtime filter" could potenti
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.h@107
PS5, Line 107:
> Can you add a blank line between methods?
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.cc
File be/src/exec/filter-context.cc:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.cc@422
PS5, Line 422: col_type, filter_expr->type().ToIR(codegen), 
"expr_type_arg");
> Why is decimal special here?
Decimal isn't supported for Kudu, so I didn't implement min-max filters for it 
since they wouldn't be testable.

I special cased it here because you might have a decimal bloom filter and you 
would hit this code path, but a better solution is to just check if the filter 
desc says to build a min-max filter, which will never be the case if the type 
is decimal.


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/filter-context.cc@434
PS5, Line 434: // Call Insert() on the bloom filter.
> Can we make this a lookup table? Seems like the only difference between the
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/hdfs-parquet-scanner.cc@401
PS5, Line 401: if (filter->BloomAlwaysTrue()
> Can we phrase the method in a way that doesn't depend on the assumption tha
I changed this to highlight the fact that the hdfs scanner only deals with 
bloom filters for now.

The runtime filter here might have a min-max filter, if it also has a kudu scan 
target, and since min-max filters don't really have a concept of "AlwaysTrue" 
we would have to say the filter isn't always true.

That would make sense if we were actually applying the min-max filter here, but 
its sort of weird since we're only really applying the bloom filter.

It can then be changed to not be bloom specific when we add consideration of 
min-max filters here.


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-scanner.cc@174
PS5, Line 174: for (const FilterContext& ctx : scan_node_->filter_ctxs_) {
> How important is this and is it blocked by anything else? If it's impactful
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-scanner.cc@175
PS5, Line 175: filt
> nullptr
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-scanner.cc@177
PS5, Line 177: se()) {
> dynamic_cast seems unnecessary since the type should always be correct - ca
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-scanner.cc@179
PS5, Line 179: eCurre
> const string&?
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-util.cc
File be/src/exec/kudu-util.cc:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/kudu-util.cc@231
PS5, Line 231:   int64_t ts_micros;
> This timestamp code is a bit subtle. Is there a sane way to combine the con
Done


http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/partitioned-hash-join-builder.cc
File be/src/exec/partitioned-hash-join-builder.cc:

http://gerrit.cloudera.org:8080/#/c/7793/5/be/src/exec/partitioned-hash-join-builder.cc@548
PS5, Line 548: IMER(repartition_ti
> Is this meant to double-count the filters if there is a bloom and min-max f
You're right, this needs to be counting "runtime filters" (corresponding to a 
single filter id/FilterContext).

This gets a little confusing, since you might have a bloom filter that gets 
disabled but its not counted as disabled here because the min-max filter isn't 
disabled, but it'll make a lot more sense in the long run goal here of having 
all scanners support all filter types (and of course the fact that the bloom 
filter is disabled in that situation will still show up in the runtime profile

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> Please see my explanation on what "incremental" stats is in previous patch
fair pointer for me, but my comment is about whether this wording is clear for 
the reader.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:38:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Greg Rahn (Code Review)

Greg Rahn has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8190 )

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:38:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu

2017-10-06 Thread Thomas Tauber-Marshall (Code Review)

Hello Michael Ho, Lars Volker, Matthew Jacobs, Tim Armstrong, Mostafa Mokhtar,

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

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

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

Change subject: IMPALA-4252: Min-max runtime filters for Kudu
..

IMPALA-4252: Min-max runtime filters for Kudu

This patch implements min-max filters for runtime filters. Each
runtime filter generates a bloom filter and/or a min-max filter,
depending on if it has HDFS and/or Kudu targets, respectively.

Min-max filters are generated by the PartitionedHashJoinBuilder. For
now, min-max filters are only applied at the KuduScanner, which passes
them into the Kudu client. Because the Kudu client doesn't provide a
way to specify generic filter exprs, min-max filters are only
generated when the target expr is a bare Kudu column ref.

Future work will address applying min-max filters at HDFS scan nodes
and applying bloom filters at Kudu scan nodes.

Codegen is used to eliminate branching on the type of the min-max
filter.

Testing:
- Updated planner tests.
- Ran existing runtime filter tests.
- Ran preliminary perf tests to demonstrate that it works. Will update
  with more specific results.
- Still needs more e2e tests.

Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/exec/filter-context.cc
M be/src/exec/filter-context.h
M be/src/exec/hdfs-parquet-scanner-ir.cc
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/kudu-scan-node-base.cc
M be/src/exec/kudu-scan-node-mt.cc
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scanner.cc
M be/src/exec/kudu-scanner.h
M be/src/exec/kudu-util.cc
M be/src/exec/kudu-util.h
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-filter-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/runtime-filter-bank.cc
M be/src/runtime/runtime-filter-bank.h
M be/src/runtime/runtime-filter-ir.cc
M be/src/runtime/runtime-filter.cc
M be/src/runtime/runtime-filter.h
M be/src/runtime/runtime-filter.inline.h
M be/src/service/impala-internal-service.cc
M be/src/util/CMakeLists.txt
A be/src/util/min-max-filter-ir.cc
A be/src/util/min-max-filter.cc
A be/src/util/min-max-filter.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-update.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
A testdata/workloads/functional-query/queries/QueryTest/bloom_filters.test
A testdata/workloads/functional-query/queries/QueryTest/bloom_filters_wait.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
M 
testdata/workloads/functional-query/queries/QueryTest/runtime_filters_wait.test
M tests/common/impala_test_suite.py
M tests/query_test/test_runtime_filters.py
M tests/util/test_file_parser.py
47 files changed, 1,581 insertions(+), 377 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
Gerrit-Change-Number: 7793
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5529: [DOCS] New trunc() signatures

2017-10-06 Thread Greg Rahn (Code Review)

Greg Rahn has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8189 )

Change subject: IMPALA-5529: [DOCS] New trunc() signatures
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4753dee4f7b8e09c35508a9cad1e36f4ab2826
Gerrit-Change-Number: 8189
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:37:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1228
PS2, Line 1228: DROP STATS and
  : DROP INCREMENTAL STATS)
> are these drops required?
They are not required if you *exactly* what you are doing, but that does not 
apply to most people. Dropping first is always safe and definitely the 
recommended practice.

If you do not drop and switch back-and-forth between compute stats and compute 
incremental sats, then you may end up with "unexpected" table metadata.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: be cached on every impalad host. If 
this metadata for a table exceeds
> more specifically, impalads that are also coordinators?
Yes


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: metadata for a table exceeds
  : 2 GB
> is there a diagnostic page that we can point to here that explains how to f
This is a longer story. I asked Bharath to help here.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
> does that mean lack of stats has not affect on optimization or something el
Please see my explanation on what "incremental" stats is in previous patch 
sets. Dropping *incremental* stats has no effect on the row counts and NDVs 
used in query optimization.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:34:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/cast-functions-ir.cc@277
PS11, Line 277:   }
> They are exercised by different parts of be/exprs/expr-test.cc .I have sear
I mean the warning messages that you're adding here have no verification.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-test.cc@670
PS11, Line 670: *
> I can change this, but I prefer it this way actually. What is the rule, sho
Yes, we should follow the style guide so we have a consistent style, which 
overall makes the code more readable.  I'm sure everyone has something that 
they dislike about the style, but we need consistency.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.h@43
PS11, Line 43: 
> This comment was just wrong, we state <= -12-31 or <1-01-01 everywh
Okay, thanks for confirming.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.cc
File be/src/runtime/timestamp-value.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.cc@116
PS11, Line 116: (
> Done
still not fixed:
if (HasDate()...



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:27:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3504: [DOCS] Document utc timestamp()

2017-10-06 Thread Lars Volker (Code Review)

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

Change subject: IMPALA-3504: [DOCS] Document utc_timestamp()
..


Patch Set 1:

> OK, looks like the those other *utc* names are not directly
 > related. Let's confine this gerrit to strictly the utc_timestamp()
 > function.

John, will you push a new PS here or is this waiting for review?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2662fc79d588f22a24a5067429a57b3c0d0f0f0
Gerrit-Change-Number: 8190
Gerrit-PatchSet: 1
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:27:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

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


Change subject: IMPALA-2758: Change BufferedTupleStream::GetRows to returning 
multi batches
..

IMPALA-2758: Change BufferedTupleStream::GetRows to returning multi batches

This patch replaces BufferedTupleStream::GetRows with GetRowBatches,
which returns multiple batches instead of one.
BufferedTupleStrem::GetRows pins a stream and read all the rows into a
single batch, ignoring batch_size configuration. It is not a good API
and may cause problems in memory management and row batch processing.

Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
5 files changed, 43 insertions(+), 68 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3831c38994da2b69775a9809ff01de5d23584414
Gerrit-Change-Number: 8226
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1228
PS2, Line 1228: DROP STATS and
  : DROP INCREMENTAL STATS)
are these drops required?


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: be cached on every impalad host. If 
this metadata for a table exceeds
more specifically, impalads that are also coordinators?


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1243
PS2, Line 1243: metadata for a table exceeds
  : 2 GB
is there a diagnostic page that we can point to here that explains how to find 
the size of metadata (either via a sql query or a monitoring webpage)?


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1247
PS2, Line 1247: does not affect
does that mean lack of stats has not affect on optimization or something else?


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml@611
PS2, Line 611: frequently
remove


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml@623
PS2, Line 623: is a shortcut
I don't know what "shortcut" means here. I'd remove it.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml
File docs/topics/impala_perf_stats.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml@361
PS2, Line 361: That situation is where you switch
I'd reword this part ("That situation is where ..."). Suggestion:

>From  and higher, use the new feature to 
>compute statistics incrementally on just the partitions that changed. See ...


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml@412
PS2, Line 412: >COMPUTE INCREMENTAL STAT
docs in impala_common mention "drop stats" before making a switch. that's not 
mentioned here. what is the required/suggested usage? perhaps call this out as 
a "switch" and link to why the user should avoid this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 06 Oct 2017 17:11:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax
..

IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

ALTER TABLE syntax for changing Kudu column default
and storage attributes.

Also SET COMMENT for non-Kudu tables.

Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Reviewed-on: http://gerrit.cloudera.org:8080/8191
Reviewed-by: Todd Lipcon 
Reviewed-by: Thomas Tauber-Marshall 
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_alter_table.xml
1 file changed, 69 insertions(+), 2 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, but someone else must approve
  Thomas Tauber-Marshall: Looks good to me, but someone else must approve
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Gerrit-Change-Number: 8191
Gerrit-PatchSet: 5
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon

[Impala-ASF-CR] IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Gerrit-Change-Number: 8191
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:45:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/161/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Gerrit-Change-Number: 8191
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:38:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

2017-10-06 Thread Michael Brown (Code Review)

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

Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..


Patch Set 1:

Thanks for fast turnaround, everyone.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:03:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:03:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

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

Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..


Patch Set 1: Code-Review+2

Lesson learned: Other components should revert immediately. PhilZ is right of 
course :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:02:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

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

Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:00:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

2017-10-06 Thread Michael Brown (Code Review)

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


Change subject: IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"
..

IMPALA-6021: Revert "IMPALA-6009: Upgrade Guava to 14.0.1"

This reverts commit 3c870aa360c5c2447ba0412e7758e3d78333110a.

Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
---
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java
2 files changed, 11 insertions(+), 10 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idd07d9b011ebd407d7a274725a10b6371d024186
Gerrit-Change-Number: 8225
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown

[Impala-ASF-CR] IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

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

Change subject: IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Gerrit-Change-Number: 8191
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 06 Oct 2017 15:21:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax

2017-10-06 Thread Thomas Tauber-Marshall (Code Review)

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

Change subject: IMPALA-4622: [DOCS] New Kudu ALTER TABLE syntax
..


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8191/4/docs/topics/impala_alter_table.xml
File docs/topics/impala_alter_table.xml:

http://gerrit.cloudera.org:8080/#/c/8191/4/docs/topics/impala_alter_table.xml@66
PS4, Line 66: ALTER TABLE name ALTER [COLUMN] 
column_name
> Just to confirm, no doc change needed as a result of this exchange?
Correct, no change needed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7647dfe8acfdb598caf561d41d74e38a8b66ce9d
Gerrit-Change-Number: 8191
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 06 Oct 2017 14:56:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

2017-10-06 Thread Csaba Ringhofer (Code Review)

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7954 )

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..


Patch Set 11:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/cast-functions-ir.cc@277
PS11, Line 277:   }
> all of these new if-stmts need end-to-end tests to cover. they aren't exerc
They are exercised by different parts of be/exprs/expr-test.cc .I have searched 
for "TestIsNull.*as timestamp", and the existing tests cover every code line, 
but not every type instantiation for CAST_TO_SUBSECOND_TIMESTAMP / 
CAST_TO_TIMESTAMP, only BIGINT/DECIMAL/DOUBLE. Actually most integer types can 
not represent values that are outside the valid range, because it needs >32 bit.

This design (BE unit tests call FE to parse expressions) seems strange to me, 
but there is probably a valid reason why it was not done in FE or E2E.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/exprs/timestamp-functions-ir.cc@139
PS11, Line 139:   }
> also needs test
Done


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-test.cc@670
PS11, Line 670: *
> nit: missing space here and below
I can change this, but I prefer it this way actually. What is the rule, should 
we follow the style guide even if the contributor consciously ignored it?  It 
doesn`t matter in this case, but it would matter with short variable names, 
e.g. I find a*a + 2*a*b much more readable than a * a + 2 * a * b.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.h@43
PS11, Line 43: 
> is that what the documentation says? and did this code change affect the up
This comment was just wrong, we state <= -12-31 or <1-01-01 everywhere 
else. This limitation comes from boost::gregorian, and this patch has no effect 
on it.


http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.cc
File be/src/runtime/timestamp-value.cc:

http://gerrit.cloudera.org:8080/#/c/7954/11/be/src/runtime/timestamp-value.cc@116
PS11, Line 116: (
> style
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 06 Oct 2017 13:00:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

2017-10-06 Thread Csaba Ringhofer (Code Review)

Hello Lars Volker, Dan Hecht,

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

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

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..

IMPALA-5664: Unix time to timestamp conversions may crash Impala

TimestampValue::FromSubsecondUnixTime() and UtcFromUnixTimeMicros()
are incorrect only in case of the last second of 1399, because these
sub-second values are rounded first towards 1400-01-01 00:00:00, which
is accepted as a valid date, and the sub-second part is subtracted
afterwards, leading to a date outside the valid interval. The maximum
case, -12-31 59:59:59 is a bit different, because as I understand,
with nanosecond precision posix times, the maximum value is actually
1-01-01. 00:00:00 minus 1 nanosec.

TimestampValue::FromUnixTimeNanos() can create problematic TimestampValues
both <1400 and 1<=.

These timestamps can cause problems, because most code assumes that if
HasDate/HasTime is true, then it really is a valid timestamp.

To fix this, the posix times are checked in the constructor of
TimestampValue, and if it is outside the valid interval,both time_
and date_ are set to not_a_date_time.

Also added logging to "cast to timestamp" functions.

Test:
select cast(-17987443200-0.1 as timestamp);
This query no longer crashes, but returns NULL, similarly to other
< 1400 timestamps.

Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
---
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-stats.inline.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/timestamp-functions-ir.cc
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
8 files changed, 81 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE

2017-10-06 Thread Attila Jeges (Code Review)

Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7564 )

Change subject: IMPALA-3548: Prune runtime filters based on query options in 
the FE
..


Patch Set 10:

> (5 comments)
 >
 > Nice. So much better. I'll let Alex take a look as well and do the
 > final +2.

Thanks for the review!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 10
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 10:29:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE

2017-10-06 Thread Attila Jeges (Code Review)

Attila Jeges has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/7564 )

Change subject: IMPALA-3548: Prune runtime filters based on query options in 
the FE
..

IMPALA-3548: Prune runtime filters based on query options in the FE

Currently, the FE generates a number of runtime filters and assigns
them to the single node plan without taking the value of
RUNTIME_FILTER_MODE and DISABLE_ROW_RUNTIME_FILTERING query options
into account.

The backend then removes filters from exec nodes, based on the
following rules:
1. If DISABLE_ROW_RUNTIME_FILTERING is set, filters are removed from
   the exec nodes that are marked as targets not bound by partition
   columns.

2. If RUNTIME_FILTER_MODE is set to LOCAL, filters are removed from
   the exec nodes that are marked as remote targets.

This may cause some confusion to users because they may see runtime
filters in the output of explain that are not applied when the query
is executed.

This change moves the logic of runtime filter pruning to the planner
in the FE. The runtime filter assignment is done on the distributed
plan and the above constraints are enforced there directly.

Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
---
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/scan-node.cc
M be/src/runtime/coordinator.cc
M be/src/service/fe-support.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
A 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-pruning.test
13 files changed, 743 insertions(+), 108 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 10
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-3548: Prune runtime filters based on query options in the FE

2017-10-06 Thread Attila Jeges (Code Review)

Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7564 )

Change subject: IMPALA-3548: Prune runtime filters based on query options in 
the FE
..


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java
File fe/src/main/java/org/apache/impala/service/FeSupport.java:

http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/main/java/org/apache/impala/service/FeSupport.java@292
PS9, Line 292: paresResult
> I believe you meant parseResult?
Yes, fixed it.


http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java
File fe/src/test/java/org/apache/impala/testutil/TestFileParser.java:

http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@125
PS9, Line 125: public int getStartingLineNum() {
 :   return startLineNum;
 : }
> single line
Done


http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@129
PS9, Line 129: public TQueryOptions getOptions() {
 :   return this.options;
 : }
> nit: single line
Done


http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@133
PS9, Line 133: public void setOptions(TQueryOptions options) {
 :   this.options = options;
 : }
> single line
Done


http://gerrit.cloudera.org:8080/#/c/7564/9/fe/src/test/java/org/apache/impala/testutil/TestFileParser.java@367
PS9, Line 367:
> Add a Preconditions.checkNotNull(result) here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f0b200e02442edcad8df3979f652d66c6e52eb
Gerrit-Change-Number: 7564
Gerrit-PatchSet: 9
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 06 Oct 2017 10:18:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4670: Introduces RpcMgr class

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-4670: Introduces RpcMgr class
..

IMPALA-4670: Introduces RpcMgr class

This patch introduces a new class, RpcMgr which is the abstraction
layer around KRPC core mechanics. It provides an interface
RegisterService() for various services to register themselves.

Kudu RPC is invoked via an auto-generated interface called proxy.
This change implements an inline wrapper for KRPC client to obtain
a proxy for a particular service exported by remote server.

Last but not least, the RpcMgr will start all registered services
if FLAGS_use_krpc is true. This patch hasn't yet added any service
except for some test services in rpc-mgr-test.

This patch is based on an abandoned patch by Henry Robinson.

Testing done: a new backend test is added to exercise the code
and demonstrate the way to interact with KRPC framework.

Change-Id: I8adb10ae375d7bf945394c38a520f12d29cf7b46
Reviewed-on: http://gerrit.cloudera.org:8080/7901
Reviewed-by: Michael Ho 
Tested-by: Impala Public Jenkins
---
M CMakeLists.txt
M be/src/exec/kudu-util.h
M be/src/rpc/CMakeLists.txt
A be/src/rpc/rpc-mgr-test.cc
A be/src/rpc/rpc-mgr.cc
A be/src/rpc/rpc-mgr.h
A be/src/rpc/rpc-mgr.inline.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/impala-server.cc
M be/src/util/counting-barrier.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
M cmake_modules/FindKRPC.cmake
A common/protobuf/CMakeLists.txt
A common/protobuf/rpc_test.proto
19 files changed, 780 insertions(+), 51 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8adb10ae375d7bf945394c38a520f12d29cf7b46
Gerrit-Change-Number: 7901
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-2190: [DOCS] from timestamp() and to timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-2190: [DOCS] from_timestamp() and to_timestamp()
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6dfd3fb99a70975d712bbef6c05900eddadd27
Gerrit-Change-Number: 8046
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Andre Calfa 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Fri, 06 Oct 2017 06:51:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2190: [DOCS] from timestamp() and to timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-2190: [DOCS] from_timestamp() and to_timestamp()
..

IMPALA-2190: [DOCS] from_timestamp() and to_timestamp()

These functions have been around for a while but didn't
get picked up in the big wave of built-in functions
documented in Impala 2.3.
Change-Id: I5f6dfd3fb99a70975d712bbef6c05900eddadd27
(cherry picked from commit 60b7540c9938c6d4fcff26d610cd8c0b0ef6cbce)
Reviewed-on: http://gerrit.cloudera.org:8080/8046
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_datetime_functions.xml
1 file changed, 144 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5f6dfd3fb99a70975d712bbef6c05900eddadd27
Gerrit-Change-Number: 8046
Gerrit-PatchSet: 4
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Andre Calfa 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell

[Impala-ASF-CR] IMPALA-2190: [DOCS] from timestamp() and to timestamp()

2017-10-06 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-2190: [DOCS] from_timestamp() and to_timestamp()
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/160/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6dfd3fb99a70975d712bbef6c05900eddadd27
Gerrit-Change-Number: 8046
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Andre Calfa 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Comment-Date: Fri, 06 Oct 2017 06:46:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS

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

Change subject: [DOCS] Tighten up advice about first COMPUTE INCREMENTAL STATS
..


Patch Set 2:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1224
PS2, Line 1224: For a particular table, use either COMPUTE 
STATS or
Yes!


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1233
PS2, Line 1233: When you run COMPUTE INCREMENTAL STATS 
on a table for the first time,
I suggest some minor rephrasing to drive home the "don't switch mantra" a 
little more, see comments.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1234
PS2, Line 1234: the statistics are computed again from scratch 
regardless of whether you previously ran
regardless of whether the table has existing stats.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1236
PS2, Line 1236: for scanning the entire table when switching from 
COMPUTE STATS to
when running COMPUTE INCREMENTAL STATS for the first time on a given table.

(do not mention switching... not supposed to do that)


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1244
PS2, Line 1244: 2 GB, a serious error can occur. If only a limited 
number of partitions are actively being
If the aggregate metadata of all tables exceeds 2 GB you may experience service 
downtime (daemon crashes).

("serious error" really isn't clear to me)


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1245
PS2, Line 1245: added or inserted into, you can run COMPUTE 
INCREMENTAL STATS for the active
Sorry my phrasing might have been misleading. By "active" partitions I meant 
those partitions that are being queried (i.e. read)... if you query some 
partitions very infrequently then there is no point in keeping incremental 
stats for them.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/shared/impala_common.xml@1248
PS2, Line 1248: optimizations such as partition pruning.
such as partition pruning or join ordering.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml
File docs/topics/impala_partitioning.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_partitioning.xml@624
PS2, Line 624: subset of partitions rather than the entire table. The 
incremental nature makes it suitable for large tables
Need to be careful here because "large tables" could be misinterpreted to mean 
"tables with many partitions".

I'd prefer to avoid the word "suitable" and instead use a phrasing that states 
it enables updating the stats as partitions are added. Whether incremental 
stats is "suitable" for anything is questionable because of the huge memory 
downside.

I'd agree that incremental stats could be suitable in situations where you have 
a huge partitioned table with a small rolling window of "active" partitions, so 
you only ever need to keep incremental stats on let's say <100 partitions.


http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml
File docs/topics/impala_perf_stats.xml:

http://gerrit.cloudera.org:8080/#/c/7999/2/docs/topics/impala_perf_stats.xml@361
PS2, Line 361:   COMPUTE STATS statement might take 
hours, or even days. That situation is where you switch
Rephrase to avoid "switch" since switching is bad



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia53a6518ce5541e5c9a2cd896856ce042a599b03
Gerrit-Change-Number: 7999
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Silvius Rus 
Gerrit-Comment-Date: Fri, 06 Oct 2017 06:43:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2190: [DOCS] from timestamp() and to timestamp()