[Impala-ASF-CR](2.x) IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1: Code-Review+2

(1 comment)

Fixed trivial merge conflict for this cherry-pick to 2.x

http://gerrit.cloudera.org:8080/#/c/10423/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10423/1/be/src/runtime/coordinator.cc@789
PS1, Line 789:   if (admission_controller != nullptr) 
admission_controller->ReleaseQuery(schedule_);
merge conflict here



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10423
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 May 2018 05:43:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR](2.x) IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10423
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 May 2018 05:43:36 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Dan Hecht (Code Review)]

Hello Tim Armstrong, Impala Public Jenkins,

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

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

to review the following change.


Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..

IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

Revert "IMPALA-5384, part 2: Simplify Coordinator locking and clarify state"

This reverts commit 6ca87e46736a1e591ed7d7d5fee05b4b4d2fbb50.

Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Reviewed-on: http://gerrit.cloudera.org:8080/10412
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.h
M be/src/util/counting-barrier.h
6 files changed, 391 insertions(+), 391 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10423
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 2:

This change did not cherrypick successfully into branch 2.x. To resolve this, 
please do the cherry-pick manually and submit it to Gerrit at refs/for/2.x or 
add an exception to the branch 2.x copy of bin/ignored_commits.json. Thanks, 
your friendly bot at https://jenkins.impala.io/job/cherrypick-2.x-and-test/499/ 
.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 May 2018 04:36:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
Reviewed-on: http://gerrit.cloudera.org:8080/10358
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 396 insertions(+), 114 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 16 May 2018 02:50:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..

IMPALA-7025: ignore resources in some planner test

The issue was that the tablesample test verified the mem-estimate
number, which depends on file sizes, which can vary slightly between
data loads.

Instead of trying to tweak the test to avoid the issue, instead provide
a mechanism to ignore the exact values of resources in planner tests
where they are not significant.

Testing:
Manually modified some values in tablesample.test, made sure that the
test still passed. Manually modified the partition count in the
expected output, made sure that the test failed.

Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Reviewed-on: http://gerrit.cloudera.org:8080/10410
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
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/TestUtils.java
3 files changed, 138 insertions(+), 86 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 16 May 2018 02:23:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Wed, 16 May 2018 02:22:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..

IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

When a UDF hits a MemLimitExceeded, the query does not
immediately abort. Instead, UDFs rely on the caller
checking the query_status_ periodically. This means that
on some codepaths, UDFs can call SetMemLimitExceeded()
many times (e.g. once per row) before the query fragment
exits.

RuntimeState::SetMemLimitExceeded() currently constructs
a MemLimitExceeded Status and dumps it for each call, even
if the query has already hit an error. This is expensive
and can delay an fragment from exiting when UDFs are
repeatedly hitting MemLimitExceeded.

This changes SetMemLimitExceeded() to avoid dumping if
the query_status_ is already not ok.

Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Reviewed-on: http://gerrit.cloudera.org:8080/10364
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/runtime-state.cc
1 file changed, 10 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-5168: Codegen HASH PARTITIONED KrpcDataStreamSender::Send()

[Michael Ho (Code Review)]

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


Change subject: IMPALA-5168: Codegen HASH_PARTITIONED 
KrpcDataStreamSender::Send()
..

IMPALA-5168: Codegen HASH_PARTITIONED KrpcDataStreamSender::Send()

This change codegens the hash partitioning logic of
KrpcDataStreamSender::Send() when the partitioning strategy
is HASH_PARTITIONED. It does so by unrolling the loop which
evaluates each row against the partitioning expressions and
hashes the result. It also replaces the number of channels
of that sender with a constant at runtime.

With this change, we get reasonable speedup with some benchmarks:

++---+-++++
| Workload   | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) | 
Delta(GeoMean) |
++---+-++++
| TPCH(_300) | parquet / none / none | 20.03   | -6.44% | 13.56  | 
-7.15% |
++---+-++++

+-+---+-++++
| Workload| File Format   | Avg (s) | Delta(Avg) | 
GeoMean(s) | Delta(GeoMean) |
+-+---+-++++
| TARGETED-PERF(_300) | parquet / none / none | 58.59   | -5.56% | 12.28
  | -5.30% |
+-+---+-++++

+-+---+-++++
| Workload| File Format   | Avg (s) | Delta(Avg) | 
GeoMean(s) | Delta(GeoMean) |
+-+---+-++++
| TPCDS-UNMODIFIED(_1000) | parquet / none / none | 15.60   | -3.10% | 7.16 
  | -4.33% |
+-+---+-++++

+---+---+-++++
| Workload  | File Format   | Avg (s) | Delta(Avg) | GeoMean(s) 
| Delta(GeoMean) |
+---+---+-++++
| TPCH_NESTED(_300) | parquet / none / none | 30.93   | -3.02% | 17.46  
| -4.71% |
+---+---+-++++

Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/codegen/impala-ir.cc
M be/src/exec/data-sink.cc
M be/src/exec/data-sink.h
M be/src/exec/exchange-node.cc
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/partial-sort-node.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/sort-node.cc
M be/src/exec/topn-node.cc
M be/src/runtime/CMakeLists.txt
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/krpc-data-stream-sender-ir.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.h
M be/src/runtime/raw-value-ir.cc
M be/src/runtime/raw-value.cc
M be/src/runtime/runtime-state.h
M be/src/util/runtime-profile.h
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
24 files changed, 397 insertions(+), 99 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 16 May 2018 00:00:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

Lars Volker has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10409 )

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..

IMPALA-7032: Disable codegen for CHAR type null literals

Analogous to IMPALA-6435, we have to disable codegen for CHAR type null
literals. Otherwise we will crash in
impala::NullLiteral::GetCodegendComputeFn().

This change adds a test to make sure that the crash is fixed.

Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Reviewed-on: http://gerrit.cloudera.org:8080/10409
Reviewed-by: Lars Volker 
Tested-by: Lars Volker 
---
M be/src/exprs/null-literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 18 insertions(+), 0 deletions(-)

Approvals:
  Lars Volker: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 15 May 2018 23:59:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..

IMPALA-7003: Deflake erasure coding data loading

Erasure coding data loading is flaky in two ways:
1. HBase sometimes doesn't work because of HBase-19369
2. Nested data loading sometimes fails because the HDFS namenode cannot
   find enough good datanodes.

For problem 1, this patch enables erasure coding only on /test-warehouse
directory. For problem 2, this patch sets
dfs.namenode.redundancy.considerLoad to false, preventing namenode from
excluding heavily-loaded datanodes.

Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Reviewed-on: http://gerrit.cloudera.org:8080/10362
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M bin/impala-config.sh
M testdata/bin/create-load-data.sh
M testdata/bin/load-test-warehouse-snapshot.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
5 files changed, 44 insertions(+), 12 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

Lars Volker has removed a vote on this change.

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/10409
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 2:

This was a false positive from the recent Jenkins configuration change to 
report unit test failures. I manually inspected the Jenkins console output and 
could not see any failed tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 15 May 2018 23:59:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7022: TestQueries.test subquery: Subquery must not return more than one row

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10407 )

Change subject: IMPALA-7022: TestQueries.test_subquery: Subquery must not 
return more than one row
..

IMPALA-7022: TestQueries.test_subquery: Subquery must not return more than one 
row

TestQueries.test_subquery sometimes fails during exhaustive
tests.

In the tests we expect to catch an exception that is
prefixed by the "Query aborted:" string. The prefix is
usually added by impala_beeswax.py::wait_for_completion(),
but in rare cases it isn't added.

>From the point of the test it is irrelevant if the exception
is prefixed with "Query aborted:" or not, so I removed it
from the expected exception string.

Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Reviewed-on: http://gerrit.cloudera.org:8080/10407
Reviewed-by: Tim Armstrong 
Tested-by: Tim Armstrong 
---
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Gerrit-Change-Number: 10407
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7022: TestQueries.test subquery: Subquery must not return more than one row

[Tim Armstrong (Code Review)]

Tim Armstrong has removed a vote on this change.

Change subject: IMPALA-7022: TestQueries.test_subquery: Subquery must not 
return more than one row
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/10407
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Gerrit-Change-Number: 10407
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7022: TestQueries.test subquery: Subquery must not return more than one row

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7022: TestQueries.test_subquery: Subquery must not 
return more than one row
..


Patch Set 1: Verified+1

This hit the same Junit test thing as my build


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Gerrit-Change-Number: 10407
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:37:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5706: Spilling sort optimisations

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5706: Spilling sort optimisations
..


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.h
File be/src/runtime/sorter.h:

http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.h@193
PS8, Line 193:   /// function calculates the maximum number of runs that can be 
taken care during the
this sentence seems garbled. "can be taken care of"?


http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.h@199
PS8, Line 199:   /// round of merging.
Can you mention the invariant that we have enough reservation for this number 
of runs. E.g. "Returns at most MaxRunsInNextMerge(), so the sorter will have 
enough reservation to merge this number of runs".


http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:

http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.cc@1676
PS8, Line 1676:   num_required_buffers += var_len_pages_found ? 2 : 1;
I think we need two buffers regardless if there are var-len slots, since 
Run::Init() always allocates a var-len page.


http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.cc@1679
PS8, Line 1679:  i-1
nit: i - 1


http://gerrit.cloudera.org:8080/#/c/9943/8/tests/query_test/test_sort.py
File tests/query_test/test_sort.py:

http://gerrit.cloudera.org:8080/#/c/9943/8/tests/query_test/test_sort.py@104
PS8, Line 104: assert "TotalMergesPerformed: 1" in 
query_result.runtime_profile
Should we also set num_nodes=1 here? Otherwise it might not be robust when 
running on different clusters.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
Gerrit-Change-Number: 9943
Gerrit-PatchSet: 8
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:30:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 15 May 2018 23:27:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 23:23:58 +
Gerrit-HasComments: No

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

[Mostafa Mokhtar (Code Review)]

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

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


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG@13
PS12, Line 13: includes S3, ADLS, and local fs.
> there are todo's in this change to extend to hdfs. pls see the comment in P
Does this change the dependency on fs.s3a.block.size?
Having to rely on that parameter for generate even scan ranges is error prone.

https://www.cloudera.com/documentation/enterprise/5-9-x/topics/impala_s3.html#s3_performance



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 15 May 2018 23:15:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 5: Code-Review+2

carry


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:10:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:06:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:06:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

[Bikramjeet Vig (Code Review)]

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

Change subject: IMPALA-5216: Make admission control queuing async
..


Patch Set 8:

(3 comments)

Just wanted to get the replay regarding 'coord_' out there. Will address the 
remaining comments in the next iteration.

http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.h@172
PS7, Line 172:
 :   /// Only set for 'QUERY' and 'DML' statement types and is 
available only after Exec()
 :   /
> If you find that there are too many entry points to CRS due to the various 
TLDR: I am not sure, if we can entirely get rid of most of the confusing usage 
around coord_ with the cleanup (since I have not done that analysis yet) but 
currently I dont think it would improve things much if we abstract away access 
to coord_

Summary:

It looks like in the codebase, GetCoordinator() is called for coord_ in 3 
differnt states:

1. null
2. initialized but Exec() not called yet
3. Exec() successfully called

One way to figure these states out if we get rid of the atomic bool is:
1. null -> hold CRS::lock, check if coord_ is nullptr

2. initialized but Exec() not called yet -> the null check in #1 plus hold the 
Coordinator::lock_, add a new Coordinator ExecState(INITIALIZED, EXECUTING,...) 
and check if Coordinator->ExecState is INITIALIZED

3. Exec() successfully called, all the above plus check coord_ ExecState

We are currently only concerned with dealing with coord_ if it is in either 
state #1 or #3.
'coord_exec_started_' is currently serving that purpose, by only making it 
visible through GetCoordinator() when it reaches state #3.
If we remove 'coord_exec_started_', we would have to hold CRS::lock_ and 
Coordinator::lock_ everytime we access it.

If we decide to remove access to coord_ and abstract away/wrap calls to coord_, 
then the way calls are being made currently, we would need 6 more wrappers.

If we do both, this would still look like just refactoring and would take the 
same amount of effort to reason about any code related to these classes.


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

http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.cc@481
PS7, Line 481: ule(query_id(),
> I think it would be okay. The only reason to be hesitant is if there might
makes sense. Created a JIRA for this IMPALA-7038


http://gerrit.cloudera.org:8080/#/c/10060/7/be/src/service/client-request-state.cc@1155
PS7, Line 1155:
> Out of curiosity, which test expects FINISHED/OK after cancellation?
hs2/test_hs2.py::TestHS2::test_get_profile
(the last assert)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cf5b259afb8f5bc5c35590c94961c81ce88bf
Gerrit-Change-Number: 10060
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 23:04:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 15 May 2018 22:55:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java
File fe/src/test/java/org/apache/impala/testutil/TestUtils.java:

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@110
PS3, Line 110:   this.keyPrefix = " " + key + "=";
> Why the leading space?
This was pretty much just preserved from the previous filters - the idea is 
just that it should be a separate token rather than a suffix of a different 
token. E.g. with "size" below we don't want to accidentally ignore foo-size= 
too.


http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@154
PS3, Line 154:   boolean filterFileSize, List lineFilters) {
> I think we can get rid of the 'filterFileSize' param and change the caller
Good point.


http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@179
PS3, Line 179:   if (SCAN_RANGE_ROW_COUNT_FILTER.matches(expectedStr)) {
> Why not clump them all into one list?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:52:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 3:

(3 comments)

Looks good. Minor additional cleanup would be nice.

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java
File fe/src/test/java/org/apache/impala/testutil/TestUtils.java:

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@110
PS3, Line 110:   this.keyPrefix = " " + key + "=";
Why the leading space?


http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@154
PS3, Line 154:   boolean filterFileSize, List lineFilters) {
I think we can get rid of the 'filterFileSize' param and change the caller to 
pass the appropriate filter instead


http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@179
PS3, Line 179:   if (SCAN_RANGE_ROW_COUNT_FILTER.matches(expectedStr)) {
Why not clump them all into one list?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:30:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..

IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

Revert "IMPALA-5384, part 2: Simplify Coordinator locking and clarify state"

This reverts commit 6ca87e46736a1e591ed7d7d5fee05b4b4d2fbb50.

Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Reviewed-on: http://gerrit.cloudera.org:8080/10412
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.h
M be/src/util/counting-barrier.h
6 files changed, 391 insertions(+), 391 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:24:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6645: Enable disk spill encryption by default

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10345 )

Change subject: IMPALA-6645: Enable disk spill encryption by default
..

IMPALA-6645: Enable disk spill encryption by default

Perf:
Targeted benchmarks with a heavily spilling query on a machine
with PCLMULQDQ support show < 5% of CPU time spent in encryption and
decryption. PCLMULQDQ was introduced in AMD Bulldozer (c. 2011)
and Intel Westmere (c. 2010).

Testing:
Ran core tests with the change.

Updated the custom cluster test to exercise the non-default
configuration.

Change-Id: Iee4be2a95d689f66c3663d99e4df0fb3968893a9
Reviewed-on: http://gerrit.cloudera.org:8080/10345
Reviewed-by: Tim Armstrong 
Tested-by: Tim Armstrong 
---
M be/src/runtime/tmp-file-mgr.cc
R testdata/workloads/functional-query/queries/QueryTest/basic-spilling.test
R tests/custom_cluster/test_disk_spill_configurations.py
3 files changed, 9 insertions(+), 6 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iee4be2a95d689f66c3663d99e4df0fb3968893a9
Gerrit-Change-Number: 10345
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6645: Enable disk spill encryption by default

[Tim Armstrong (Code Review)]

Tim Armstrong has removed a vote on this change.

Change subject: IMPALA-6645: Enable disk spill encryption by default
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/10345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iee4be2a95d689f66c3663d99e4df0fb3968893a9
Gerrit-Change-Number: 10345
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6645: Enable disk spill encryption by default

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6645: Enable disk spill encryption by default
..


Patch Set 8: Verified+1

False positive from the JUnit test publisher picking up random xml files. I 
looked through the build output and didn't see any failed tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee4be2a95d689f66c3663d99e4df0fb3968893a9
Gerrit-Change-Number: 10345
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:23:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 15 May 2018 22:19:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7022: TestQueries.test subquery: Subquery must not return more than one row

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7022: TestQueries.test_subquery: Subquery must not 
return more than one row
..


Patch Set 1: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Gerrit-Change-Number: 10407
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:14:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: -Code-Review

My understanding is you and Adam are still working on this. Let me know when 
this is ready to be merged.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 22:08:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:

http://gerrit.cloudera.org:8080/#/c/10410/3/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@458
PS3, Line 458: PlannerTestOption.INCLUDE_EXPLAIN_HEADER,
Inconsistent formatting here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:08:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 3:

I changed the runPlannerTestFile() API to do a whitelist approach as suggested. 
I rolled a couple of other options into the same mechanism so each test can 
provide a list of the extra things it wants to enable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 22:07:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 396 insertions(+), 114 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

Hello Alex Behm,

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

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

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

Change subject: IMPALA-7025: ignore resources in some planner test
..

IMPALA-7025: ignore resources in some planner test

The issue was that the tablesample test verified the mem-estimate
number, which depends on file sizes, which can vary slightly between
data loads.

Instead of trying to tweak the test to avoid the issue, instead provide
a mechanism to ignore the exact values of resources in planner tests
where they are not significant.

Testing:
Manually modified some values in tablesample.test, made sure that the
test still passed. Manually modified the partition count in the
expected output, made sure that the test failed.

Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
---
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/TestUtils.java
3 files changed, 136 insertions(+), 72 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Fredy Wijaya (Code Review)]

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5:

Alex, can you please carry +2?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 22:06:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7035: Configure jceks.key.serialFilter for KMS.

[Philip Zeyliger (Code Review)]

Philip Zeyliger has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10418


Change subject: IMPALA-7035: Configure jceks.key.serialFilter for KMS.
..

IMPALA-7035: Configure jceks.key.serialFilter for KMS.

Configures a Java property for KMS to account for JDK 8u171's security fixes. I
was seeing impala-py.test tests/metadata/test_hdfs_encryption.py fail with the
following error:

  AssertionError: Error creating encryption zone: RemoteException: Can't 
recover key for testkey1 from keystore 
file:/home/impdev/Impala/testdata/cluster/cdh6/node-1/data/kms.keystore

The issue is described in HDFS-13494, and I imagine it'll be fixed in due time. 
In the
meanwhile, setting this property seems to do the trick.

Change-Id: I2d21c9cce3b91e8fd8b2b4f1cda75e3958c977d5
---
M testdata/cluster/node_templates/cdh6/etc/init.d/kms
1 file changed, 4 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d21c9cce3b91e8fd8b2b4f1cda75e3958c977d5
Gerrit-Change-Number: 10418
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 3:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@522
PS3, Line 522: // With clause with insert.s
> typo: trailing s
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 21:33:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> The objects are just passed as a string and the code appears to be identica
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 21:29:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 15 May 2018 21:09:40 +
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10398 )

Change subject: IMPALA-6957: calc thread resource requirement in planner
..

IMPALA-6957: calc thread resource requirement in planner

This only factors in fragment execution threads. E.g. this does *not*
try to account for the number of threads on the old Thrift RPC
code path if that is enabled.

This is loosely related to the old VCores estimate, but is different in
that it:
* Directly ties into the notion of required threads in
  ThreadResourceMgr.
* Is a strict upper bound on the number of such threads, rather than
  an estimate.

Does not include "optional" threads. ThreadResourceMgr in the backend
bounds the number of "optional" threads per impalad, so the number of
execution threads on a backend is limited by

  sum(required threads per query) +
  CpuInfo::num_cores() * FLAGS_num_threads_per_core

DCHECKS in the backend enforce that the calculation is correct. They
were actually hit in KuduScanNode because of some races in thread
management leading to multiple "required" threads running. Now the
first thread in the multithreaded scans never exits, which means
that it's always safe for any of the other threads to exit early,
which simplifies the logic a lot.

Testing:
Updated planner tests.

Ran core tests.

Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Reviewed-on: http://gerrit.cloudera.org:8080/10256
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
Reviewed-on: http://gerrit.cloudera.org:8080/10398
---
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/kudu-scan-node.cc
M be/src/exec/kudu-scan-node.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/thread-resource-mgr.cc
M be/src/runtime/thread-resource-mgr.h
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/Planner.thrift
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/ResourceProfile.java
M fe/src/main/java/org/apache/impala/planner/ResourceProfileBuilder.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level0.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
43 files changed, 1,973 insertions(+), 1,927 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10398
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6957: calc thread resource requirement in planner
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10398
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 21:03:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Alex Behm (Code Review)]

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

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(2 comments)

Thanks for the cleanup, much more condensed!

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> This redundancy cannot be removed since the objects are different.
The objects are just passed as a string and the code appears to be identical 
otherwise, so can't we just loop over ["alltypes", "alltypes_view"] generating 
a new AuthzTest for each?


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

http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@522
PS3, Line 522: onColumn("functional", "alltypestiny", new 
String[]{"id", "bool_col",
typo: trailing s



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 20:54:06 +
Gerrit-HasComments: Yes

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

[Mostafa Mokhtar (Code Review)]

Mostafa Mokhtar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10415


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

IMPALA-6034: Add Cpu and scanned bytes limits per query

To protect Impala from runaway queries add per query limits for Cpu and
scanned bytes, limits are enforced via query options and apply to the entire
query, not per backend like mem_limit.
If a query exceeds any of the limits it will get cancelled.

Query profile is upated to include query wide and per backend metrics for Cpu
and scanned bytes.

Testing:
Added tests for various permutations for  MAX_CPU_TIME_S and MAX_SCAN_BYTES

Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_query_expiration.py
12 files changed, 347 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 1
Gerrit-Owner: Mostafa Mokhtar 

[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 1:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/10413/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@439
PS1, Line 439:   if (synthesizeFileMd || fileStatus.isErasureCoded()) {
Shouldn't this check be in L424 so that synthesizeFileMd is true for EC? Are 
they EC blocks really completely useless to us and we're better of synthesizing 
them?

I think it's worth adding a comment explaining why we chose this path for EC.


http://gerrit.cloudera.org:8080/#/c/10413/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@510
PS1, Line 510: if (synthesizeFileMd || fileStatus.isErasureCoded()) {
move check to L488?


http://gerrit.cloudera.org:8080/#/c/10413/1/tests/common/skip.py
File tests/common/skip.py:

http://gerrit.cloudera.org:8080/#/c/10413/1/tests/common/skip.py@149
PS1, Line 149:   ec = pytest.mark.skipif(IS_EC, reason="It shouldn't work with 
erasure coding.")
Any more concrete reason, or are there too many to list?


http://gerrit.cloudera.org:8080/#/c/10413/1/tests/custom_cluster/test_hdfs_fd_caching.py
File tests/custom_cluster/test_hdfs_fd_caching.py:

http://gerrit.cloudera.org:8080/#/c/10413/1/tests/custom_cluster/test_hdfs_fd_caching.py@29
PS1, Line 29: @SkipIfEC.fix_later
Is this really expected to work? FD caching is only applied to local sc reads, 
and not used for remote reads.


http://gerrit.cloudera.org:8080/#/c/10413/1/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

http://gerrit.cloudera.org:8080/#/c/10413/1/tests/query_test/test_mt_dop.py@101
PS1, Line 101:   # Impala scans less row group than it should with erasure 
coding.
fewer row groups



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Tue, 15 May 2018 20:43:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 15 May 2018 20:35:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10410/2//COMMIT_MSG@13
PS2, Line 13: Instead of trying to tweak the test to avoid the issue, instead 
provide
> We need to test the values in the tests that are meant to validate the reso
Correct. I'm saying a whitelist may be more robust because it means new tests 
will by default not validate the mem reservation. The people adding tests 
around resources will know to enable them :) (but others may not know to 
disable them in the blacklisting approach)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 20:33:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 15 May 2018 20:30:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Philip Zeyliger (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10362/3/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
File testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl:

http://gerrit.cloudera.org:8080/#/c/10362/3/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl@122
PS3, Line 122: dfs.namenode.redundancy.considerLoad
This was a great find!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 15 May 2018 20:26:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10410/2//COMMIT_MSG@13
PS2, Line 13: Instead of trying to tweak the test to avoid the issue, instead 
provide
> Why not ignore them everywhere instead of the tests that are specifically d
We need to test the values in the tests that are meant to validate the resource 
calculations. We will need to tweak or design those tests so that they're 
robust to slight changes in file size or data layout, but this approach means 
we don't need to tweak tests like testTableSample unnecessarily.

Or are you saying that we should whitelist tests that are explicitly checking 
them?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 20:07:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6983: stress: don't write a null runtime profile

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6983: stress: don't write a null runtime profile
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8299a8a97ad1f2bd1f2927e3111db8df1d3a3e5
Gerrit-Change-Number: 10381
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Tue, 15 May 2018 20:05:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Joe McDonnell (Code Review)]

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10364 )

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10364/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:

http://gerrit.cloudera.org:8080/#/c/10364/1/be/src/runtime/runtime-state.cc@210
PS1, Line 210:   // (e.g. once per row) before the fragment aborts. See 
IMPALA-6997.
> I wonder if we should explicitly check that the existing error is TErrorCod
We usually record the first error that we hit. If the query status is not ok, 
then it is aborting. I think it is ok to skip this if we already hit a 
different error.


http://gerrit.cloudera.org:8080/#/c/10364/1/be/src/runtime/runtime-state.cc@211
PS1, Line 211:   if (!query_status_.ok()) return;
> This probably works in practice but we don't guarantee that Status is threa
Changed the code to hold the lock for this check. We can revisit this when we 
nail down the Status semantics.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 15 May 2018 19:51:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

[Joe McDonnell (Code Review)]

Hello Zoram Thanga, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()
..

IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded()

When a UDF hits a MemLimitExceeded, the query does not
immediately abort. Instead, UDFs rely on the caller
checking the query_status_ periodically. This means that
on some codepaths, UDFs can call SetMemLimitExceeded()
many times (e.g. once per row) before the query fragment
exits.

RuntimeState::SetMemLimitExceeded() currently constructs
a MemLimitExceeded Status and dumps it for each call, even
if the query has already hit an error. This is expensive
and can delay an fragment from exiting when UDFs are
repeatedly hitting MemLimitExceeded.

This changes SetMemLimitExceeded() to avoid dumping if
the query_status_ is already not ok.

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701
Gerrit-Change-Number: 10364
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 409 insertions(+), 98 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 

[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

[Tianyi Wang (Code Review)]

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


Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..

IMPALA-7019: Schedule EC as remote & disable failed tests

This patch schedules HDFS EC files without considering locality. Failed
tests are disabled and a jenkins build should succeed with export
ERASURE_COINDG=true.

Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
---
M common/fbs/CatalogObjects.fbs
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/common/skip.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_mt_dop.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_queries.py
M tests/query_test/test_query_mem_limit.py
M tests/query_test/test_scanners.py
M tests/util/filesystem_utils.py
15 files changed, 48 insertions(+), 15 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7003: Deflake erasure coding data loading

[Tianyi Wang (Code Review)]

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

Change subject: IMPALA-7003: Deflake erasure coding data loading
..

IMPALA-7003: Deflake erasure coding data loading

Erasure coding data loading is flaky in two ways:
1. HBase sometimes doesn't work because of HBase-19369
2. Nested data loading sometimes fails because the HDFS namenode cannot
   find enough good datanodes.

For problem 1, this patch enables erasure coding only on /test-warehouse
directory. For problem 2, this patch sets
dfs.namenode.redundancy.considerLoad to false, preventing namenode from
excluding heavily-loaded datanodes.

Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
---
M bin/impala-config.sh
M testdata/bin/create-load-data.sh
M testdata/bin/load-test-warehouse-snapshot.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
5 files changed, 44 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I219106cd3ec7ffab7a834700f2a722b165e5f66c
Gerrit-Change-Number: 10362
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR](2.x) IMPALA-6957: calc thread resource requirement in planner

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6957: calc thread resource requirement in planner
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I982837ef883457fa4d2adc3bdbdc727353469140
Gerrit-Change-Number: 10398
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 19:24:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10410/2//COMMIT_MSG@13
PS2, Line 13: Instead of trying to tweak the test to avoid the issue, instead 
provide
Why not ignore them everywhere instead of the tests that are specifically 
designed to check them? If the issue is related to data loading, then somebody 
might add a new planner test that breaks. It's not commented or documented 
anywhere why some tests ignore the mem estimates and others don't, so other 
people will not easily know whether to ignore or not when authoring new tests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
Gerrit-Change-Number: 10410
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Tue, 15 May 2018 19:12:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 18:52:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 18:52:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10412/1/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

http://gerrit.cloudera.org:8080/#/c/10412/1/be/src/runtime/coordinator.h@45
PS1, Line 45: #include "util/runtime-profile-counters.h"
: #include "util/spinlock.h"
this was a conflict (due to my later change to make wait_lock_ a spinlock).


http://gerrit.cloudera.org:8080/#/c/10412/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10412/1/be/src/runtime/coordinator.cc@a659
PS1, Line 659:
 :
this was a conflict (due to later change to add this PrintId().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Comment-Date: Tue, 15 May 2018 18:51:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7025: ignore resources in some planner test

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7025: ignore resources in some planner test
..

IMPALA-7025: ignore resources in some planner test

The issue was that the tablesample test verified the mem-estimate
number, which depends on file sizes, which can vary slightly between
data loads.

Instead of trying to tweak the test to avoid the issue, instead provide
a mechanism to ignore the exact values of resources in planner tests
where they are not significant.

Testing:
Manually modified some values in tablesample.test, made sure that the
test still passed. Manually modified the partition count in the
expected output, made sure that the test failed.

Change-Id: I91e3e416ec6242fbf22d9f566fdd1ce225cb16ac
---
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/TestUtils.java
3 files changed, 91 insertions(+), 45 deletions(-)


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

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

[Impala-ASF-CR] IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

[Dan Hecht (Code Review)]

Dan Hecht has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10412


Change subject: IMPALA-7033/IMPALA-7030: Backout suspected change leading to 
crash
..

IMPALA-7033/IMPALA-7030: Backout suspected change leading to crash

Revert "IMPALA-5384, part 2: Simplify Coordinator locking and clarify state"

This reverts commit 6ca87e46736a1e591ed7d7d5fee05b4b4d2fbb50.

Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
---
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.h
M be/src/util/counting-barrier.h
6 files changed, 391 insertions(+), 391 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc63006e6e04130b2873a6a9730e434c563327c5
Gerrit-Change-Number: 10412
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 

[Impala-ASF-CR] IMPALA-6827: [DOCS] Updated the download link for the tutorial data

[Alex Rodoni (Code Review)]

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

Change subject: IMPALA-6827: [DOCS] Updated the download link for the tutorial 
data
..


Patch Set 2:

> > > Safety is one thing, control is another. We don't have control
 > > over
 > > > that site, so we can't control their decision to move or take
 > > down
 > > > the data.
 > >
 > > We don't have control over the native-toolchain bucket, either.
 > > That's controlled by Cloudera, not by Apache Impala.
 > >
 > > I'd suggest speaking to ASF infra about hosting a large binary
 > like
 > > this. They might have a turn-key solution.
 >
 > Wanted to make sure this didn't get lost in the fray. Alex, you can
 > hop onto https://infra.chat to talk directly to ASF infra in
 > real-time.

 > (2 comments)

Yup, working on it with asf infra.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6823d1688169e0a6f09d5b552026bc18a3770828
Gerrit-Change-Number: 10393
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 15 May 2018 18:41:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7029: Clone LHS when rewriting a between predicate

[Alex Behm (Code Review)]

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

Change subject: IMPALA-7029: Clone LHS when rewriting a between predicate
..


Patch Set 2:

Tianyi, before we pursue this further let's get together for me to understand 
what exactly is going wrong.

There should be no requirement to have different expr objects in different expr 
trees. Arguably, having such a guarantee would be cleaner, but I'd like to 
understand if there are other subtle issues lurking in the current model and 
whether we need a more fundamental change.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2db3af99593bc207ee8dc2a1550acf21ebd61c41
Gerrit-Change-Number: 10402
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 15 May 2018 18:39:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7022: TestQueries.test subquery: Subquery must not return more than one row

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7022: TestQueries.test_subquery: Subquery must not 
return more than one row
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b8655ad273b1dd7a601099f617db609e4a4797b
Gerrit-Change-Number: 10407
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 18:40:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6827: [DOCS] Updated the download link for the tutorial data

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10393 )

Change subject: IMPALA-6827: [DOCS] Updated the download link for the tutorial 
data
..


Patch Set 2:

> > Safety is one thing, control is another. We don't have control
 > over
 > > that site, so we can't control their decision to move or take
 > down
 > > the data.
 >
 > We don't have control over the native-toolchain bucket, either.
 > That's controlled by Cloudera, not by Apache Impala.
 >
 > I'd suggest speaking to ASF infra about hosting a large binary like
 > this. They might have a turn-key solution.

Wanted to make sure this didn't get lost in the fray. Alex, you can hop onto 
https://infra.chat to talk directly to ASF infra in real-time.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6823d1688169e0a6f09d5b552026bc18a3770828
Gerrit-Change-Number: 10393
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Harsh J 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Tue, 15 May 2018 18:37:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 15 May 2018 18:35:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 2: Code-Review+2

(1 comment)

Thanks for the quick review. I addressed your comment in PS2.

Carrying Michael's +2.

http://gerrit.cloudera.org:8080/#/c/10409/1/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
File testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test:

http://gerrit.cloudera.org:8080/#/c/10409/1/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test@48
PS1, Line 48: 
> nit: extra 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 15 May 2018 18:34:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

Hello Michael Ho,

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

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

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..

IMPALA-7032: Disable codegen for CHAR type null literals

Analogous to IMPALA-6435, we have to disable codegen for CHAR type null
literals. Otherwise we will crash in
impala::NullLiteral::GetCodegendComputeFn().

This change adds a test to make sure that the crash is fixed.

Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
---
M be/src/exprs/null-literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 18 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Michael Ho (Code Review)]

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

Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10409/1/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
File testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test:

http://gerrit.cloudera.org:8080/#/c/10409/1/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test@48
PS1, Line 48: 
nit: extra 



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
Gerrit-Change-Number: 10409
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Tue, 15 May 2018 18:27:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7032: Disable codegen for CHAR type null literals

[Lars Volker (Code Review)]

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


Change subject: IMPALA-7032: Disable codegen for CHAR type null literals
..

IMPALA-7032: Disable codegen for CHAR type null literals

Analogous to IMPALA-6435, we have to disable codegen for CHAR type null
literals. Otherwise we will crash in
impala::NullLiteral::GetCodegendComputeFn().

This change adds a test to make sure that the crash is fixed.

Change-Id: I34033362263cf1292418f69c5ca1a3b84aed39a9
---
M be/src/exprs/null-literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 19 insertions(+), 0 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

[Adam Holley (Code Review)]

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(9 comments)

Taking over for Fredy while he's out.

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@96
PS2, Line 96: Set expectedAuthorizables = Sets.newHashSet(
> Let's factor this out somewhere, it's repeated many times
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@718
PS2, Line 718: authorize("with t as (select id from functional.alltypes) 
select * from t")
> Tests are fine, but overall there's still a lot of redundancy. Basically an
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@736
PS2, Line 736: authorize("with t as (select id, int_col, 2019 from 
functional.alltypesagg) " +
> Same here, this is basically the same as INSERT (without WITH)
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@772
PS2, Line 772: authorize("select id from functional.alltypes union all " +
> Same as SELECT with a join... also redundancy here
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@842
PS2, Line 842: for (AuthzTest test : new AuthzTest[]{
> This is an interesting pattern for handling the redundancy I mentioned abov
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> redundancy with L842
This redundancy cannot be removed since the objects are different.


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@880
PS2, Line 880: .error(refreshError("functional"), 
onDatabase("functional", allExcept(
> also error with onServer()
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@917
PS2, Line 917: // Show partitions.
> Many of these require db or table level show metadata privs. Also add negat
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1053
PS2, Line 1053: return new TPrivilegeLevel[]{TPrivilegeLevel.INSERT, 
TPrivilegeLevel.INSERT,
> INSERT listed twice, is SELECT missing?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 18:06:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put ASAN options in CMakeLists.

[Tim Armstrong (Code Review)]

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

Change subject: Put ASAN options in CMakeLists.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10404/1//COMMIT_MSG@10
PS1, Line 10: to avoid various failures.  In particular, backend tests fail
> Yes, I'm confused here. My testing missed something; I'm figuring out what.
a) seems best to me. We should mention ASAN_OPTIONS in an adjacent comment 
though so people can find it via grep if they want to override or understand 
where the values come from.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbd210c67750a48003f336bea1f3e1cb2d9e6b
Gerrit-Change-Number: 10404
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 18:01:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put ASAN options in CMakeLists.

[Philip Zeyliger (Code Review)]

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

Change subject: Put ASAN options in CMakeLists.
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10404/1//COMMIT_MSG@10
PS1, Line 10: to avoid various failures.  In particular, backend tests fail
> Maybe I'm missing something, but why does setting this in CMake help? I don
Yes, I'm confused here. My testing missed something; I'm figuring out what. 
It's possible that the variables percolate to "make test", but that's neither 
here nor there.

The Jenkins job I was looking at does:

$git grep ASAN_OPTIONS
jenkins/build.sh:export ASAN_OPTIONS="${ASAN_OPTIONS} handle_segv=0"
jenkins/build.sh:export ASAN_OPTIONS="${ASAN_OPTIONS} detect_leaks=0"

Meanwhile, we always start our daemons with ASAN_OPTIONS.

$git grep ASAN_OPTIONS
bin/run-backend-tests.sh:export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 
allocator_may_return_null=1"
bin/start-catalogd.sh:export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 
allocator_may_return_null=1"
bin/start-impalad.sh:export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 
allocator_may_return_null=1"
bin/start-statestored.sh:export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 
allocator_may_return_null=1"

I have a few ways forward:

a) Implement const char *__asan_default_options() in our C++ code and have it 
linked everywhere. It's tempting, and I could get rid of the environment 
variables.

b) Add ASAN_OPTIONS to some variant of bin/run-all-tests.sh or impala-config.sh.

c) In the callout to parquet-reader in tests/query_test/test_insert_parquet.py, 
set the environment variable explicitly.


Do you have a preference? I can't decide.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbd210c67750a48003f336bea1f3e1cb2d9e6b
Gerrit-Change-Number: 10404
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 17:58:53 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3134: Support different proc mem limits among impalads for admission control checks

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3134: Support different proc mem limits among impalads 
for admission control checks
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10396/2/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/10396/2/bin/start-impala-cluster.py@87
PS2, Line 87: seperated
separated



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb72eee790cc17466bbfa82e30f369a65f2b060e
Gerrit-Change-Number: 10396
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 17:50:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3134: Support different proc mem limits among impalads for admission control checks

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3134: Support different proc mem limits among impalads 
for admission control checks
..


Patch Set 2:

(10 comments)

Looks good, had some minor comments.

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

http://gerrit.cloudera.org:8080/#/c/10396/2//COMMIT_MSG@14
PS2, Line 14: decisions.
Can you mention that this assumes that the per-process memory limit does not 
change dynamically? Might not be obvious.


http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/admission-controller.cc@49
PS2, Line 49: int64_t GetProcMemLimit() {
This is dead code now


http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/admission-controller.cc@152
PS2, Line 152: const string HOST_MEM_NOT_AVAILABLE = "Not enough memory 
available on host $0."
Maybe this error message should include the total process memory limit, e.g. 
"$2/$3 was available"


http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/admission-controller.cc@450
PS2, Line 450:
Unnecessary vertical whitespace


http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/admission-controller.cc@458
PS2, Line 458:
Vertical whitespace


http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:

http://gerrit.cloudera.org:8080/#/c/10396/2/be/src/scheduling/query-schedule.h@69
PS2, Line 69:   // The process memory limit of this backend.
Maybe mention that it's the value obtained from the statestore during 
scheduling. We don't expect it to change right now, but that may be important 
context if we ever want to change to dynamic process memory limits.


http://gerrit.cloudera.org:8080/#/c/10396/2/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/10396/2/tests/custom_cluster/test_admission_controller.py@429
PS2, Line 429: 1000
Can we set this lower? Is there a reason that it needs to sit in the queue for 
1s?


http://gerrit.cloudera.org:8080/#/c/10396/2/tests/custom_cluster/test_admission_controller.py@431
PS2, Line 431:   def test_heterogeneous_proc_mem_limit(self, vector):
Can we add a 3GB query that succeeds? E.g. with num_nodes=1, submitted to one 
of the 3GB nodes.


http://gerrit.cloudera.org:8080/#/c/10396/2/tests/custom_cluster/test_admission_controller.py@433
PS2, Line 433: mem limits of each impalad"""
Maybe add an extra sentence to explain the concept of the test. e.g. "Starts a 
cluster where the first impalad has a smaller proc mem limit than other 
impalads and run queries where admission/reject depends on the coordinator 
knowing the other impalad's mem limits".


http://gerrit.cloudera.org:8080/#/c/10396/2/tests/custom_cluster/test_admission_controller.py@434
PS2, Line 434: # Choose a query that runs on all 3 backends.
Are all these queries submitted to the first impalad? It's not clear if that's 
the intent.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb72eee790cc17466bbfa82e30f369a65f2b060e
Gerrit-Change-Number: 10396
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 17:44:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5384: Simplify coordinator locking protocol

[Dan Hecht (Code Review)]

Dan Hecht has abandoned this change. ( http://gerrit.cloudera.org:8080/7065 )

Change subject: IMPALA-5384: Simplify coordinator locking protocol
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6a79aa38e529d0994921906b2beb796fd589a7e3
Gerrit-Change-Number: 7065
Gerrit-PatchSet: 2
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 

[Impala-ASF-CR] IMPALA-5384: Simplify coordinator locking protocol

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-5384: Simplify coordinator locking protocol
..


Patch Set 2:

This has been subsumed by another set of patches that were derived from this 
one.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a79aa38e529d0994921906b2beb796fd589a7e3
Gerrit-Change-Number: 7065
Gerrit-PatchSet: 2
Gerrit-Owner: Marcel Kornacker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Comment-Date: Tue, 15 May 2018 17:40:37 +
Gerrit-HasComments: No

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

[Vuk Ercegovac (Code Review)]

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

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


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG@13
PS12, Line 13: includes S3, ADLS, and local fs.
> What about erasure coded HDFS data?
there are todo's in this change to extend to hdfs. pls see the comment in 
PlanNodes.thrift.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 15 May 2018 17:35:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3307: Add support for IANA time-zone db

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-3307: Add support for IANA time-zone db
..


Patch Set 7: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93c1fbffe81f067919706e30db0a34d0e58e7e77
Gerrit-Change-Number: 9986
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 15 May 2018 17:35:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3307: Add support for IANA time-zone db

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-3307: Add support for IANA time-zone db
..


Patch Set 7:

(2 comments)

I focused mostly on the non-timestamp/timezone/time and test/infra parts. It 
looks fine to me. Would be good to get Gabor's to finish his review to +1 and 
Tim can do the final +2.

Just a heads up regarding exceptions: in the past we've had a lot of issues 
with timestamp boost routines throwing exceptions for out of range values. You 
should make sure you exercise any path that can do that with tests. We 
generally either have to reason about why the boost function can't throw an 
exception (maybe we check the range before hand) or we wrap the boost call with 
try/catch so we don't expose the exception. Also IIRC, something to keep in 
mind is that codegen code can't properly handle try/catch, so in cases we 
needed to use that, we factored the try/catch code into native code and call 
out to it from the IR. Again, just a heads up, not sure if your change 
introduced any problem in this regard or not.

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

http://gerrit.cloudera.org:8080/#/c/9986/7/be/src/exprs/timestamp-functions-ir.cc@502
PS7, Line 502:
 : namespace {
 : inline cctz::time_point 
UnixTimeToTimePoint(time_t t) {
 :   return std::chrono::time_point_cast(
 :   std::chrono::system_clock::from_time_t(0)) + 
cctz::sys_seconds(t);
 : }
 :
 : }
 :
 : StringVal TimestampFunctions::TimeOfDay(FunctionContext* 
context) {
 :   const TimestampVal curr = Now(context);
 :   if (curr.is_null) return StringVal::null();
 :   const string& day = ShortDayName(context, curr);
 :   const string& month = ShortMonthName(context, curr);
 :   IntVal dayofmonth = DayOfMonth(context, curr);
 :   IntVal hour = Hour(context, curr);
 :   IntVal min = Minute(context, curr);
 :   IntVal sec = Second(context, curr);
 :   IntVal year = Year(context, curr);
 :
 :   // Calculate 'start' time point at which query execution 
started.
 :   cctz::time_point start = 
UnixTimeToTimePoint(
 :   context->impl()->state()->query_ctx().start_unix_millis / 
MILLIS_PER_SEC);
 :   // Find 'tz_name' time-zone abbreviation that corresponds to 
'local_time_zone' at
 :   // 'start' time point.
 :   cctz::time_zone::absolute_lookup start_lookup =
 :   context->impl()->state()->local_time_zone()->lookup(start);
 :   const string& tz_name = (start_lookup.abbr != nullptr) ? 
start_lookup.abbr :
 :   context->impl()->state()->local_time_zone()->name();
any chance that can throw an exception? I believe our IR code can't properly 
handle try/catch, so if this can indeed throw an exception and needs to be 
wrapped in try/catch, it may need to be refactored so that this code lives in 
the native code and we call out to it from the IR. (Just a heads up, this may 
not be a problem here).


http://gerrit.cloudera.org:8080/#/c/9986/7/be/src/runtime/timestamp-value.inline.h
File be/src/runtime/timestamp-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/9986/7/be/src/runtime/timestamp-value.inline.h@59
PS7, Line 59: .days()
in the past we've had issues where the boost date library can throw exceptions. 
I don't remember the details off hand and it may be that you are okay here 
given you've already checked HasDateAndTime() and if we ensure date_ is within 
range, but just wanted to mention it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93c1fbffe81f067919706e30db0a34d0e58e7e77
Gerrit-Change-Number: 9986
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 15 May 2018 17:34:57 +
Gerrit-HasComments: Yes

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

[Mostafa Mokhtar (Code Review)]

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

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


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8523/12//COMMIT_MSG@13
PS12, Line 13: includes S3, ADLS, and local fs.
What about erasure coded HDFS data?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I326065adbb2f7e632814113aae85cb51ca4779a5
Gerrit-Change-Number: 8523
Gerrit-PatchSet: 12
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 15 May 2018 17:26:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

[Zoltan Borok-Nagy (Code Review)]

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

Change subject: IMPALA-5842: Write page index in Parquet files
..


Patch Set 18:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py
File tests/query_test/test_parquet_page_index.py:

http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@71
PS17, Line 71: if column_index_offset and column_index_length:
> It might make things faster / more elegant to pass an open file here, other
Done


http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@126
PS17, Line 126: es = page_header.data
> Why not "not page_is_null"?
Guess I wrote too many "is None"...
Done.


http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@179
PS17, Line 179:
> "reverse" seems to rely on everyone thinking of ascending as the default. M
The Python sorted() function also has a reversed flag with the same meaning, so 
maybe it's the Python convention. If you still think I should be more explicit, 
I'll change the name of it.

We could use BoundaryOrder, and define it to UNORDERED, but we couldn't really 
use it because BoundaryOrder says something about two lists, not just one.
I.e., testing the following in case of UNORDERED wouldn't be enough:

 is_sorted(min_values, UNORDERED) or
 is_sorted(max_values, UNORDERED)

E.g. if min_values is ascending and max_values is descending the above 
expression would evaluate to false, but the boundary order should be UNORDERED.


http://gerrit.cloudera.org:8080/#/c/9693/17/tests/util/get_parquet_metadata.py
File tests/util/get_parquet_metadata.py:

http://gerrit.cloudera.org:8080/#/c/9693/17/tests/util/get_parquet_metadata.py@163
PS17, Line 163:
> If you follow the suggestion in the other file to pass an open file to read
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 18
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Anonymous Coward #248
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 15 May 2018 16:53:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5842: Write page index in Parquet files

[Zoltan Borok-Nagy (Code Review)]

Hello Lars Volker, Anonymous Coward #248, Tim Armstrong, Csaba Ringhofer,

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

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

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

Change subject: IMPALA-5842: Write page index in Parquet files
..

IMPALA-5842: Write page index in Parquet files

This commit builds on the previous work of
Pooja Nilangekar: https://gerrit.cloudera.org/#/c/7464/

The commit implements the write path of PARQUET-922:
"Add column indexes to parquet.thrift". As specified in the
parquet-format, Impala writes the page indexes just before
the footer. This allows much more efficient page filtering
than using the same information from the 'statistics' field
of DataPageHeader.

I updated Pooja's python tests as well.

Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
---
M be/src/exec/hdfs-parquet-table-writer.cc
M be/src/exec/hdfs-parquet-table-writer.h
M be/src/exec/parquet-column-stats.h
M be/src/exec/parquet-column-stats.inline.h
M be/src/util/CMakeLists.txt
A be/src/util/string-util-test.cc
A be/src/util/string-util.cc
A be/src/util/string-util.h
M common/thrift/parquet.thrift
M testdata/bin/load-dependent-tables.sql
M tests/query_test/test_chars.py
A tests/query_test/test_parquet_page_index.py
M tests/util/get_parquet_metadata.py
13 files changed, 993 insertions(+), 108 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9
Gerrit-Change-Number: 9693
Gerrit-PatchSet: 18
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Anonymous Coward #248
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-6983: stress: don't write a null runtime profile

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-6983: stress: don't write a null runtime profile
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8299a8a97ad1f2bd1f2927e3111db8df1d3a3e5
Gerrit-Change-Number: 10381
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Tue, 15 May 2018 16:37:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6983: stress: don't write a null runtime profile

[Michael Brown (Code Review)]

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

Change subject: IMPALA-6983: stress: don't write a null runtime profile
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8299a8a97ad1f2bd1f2927e3111db8df1d3a3e5
Gerrit-Change-Number: 10381
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Tue, 15 May 2018 16:37:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6983: stress: don't write a null runtime profile

[Michael Brown (Code Review)]

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

Change subject: IMPALA-6983: stress: don't write a null runtime profile
..


Patch Set 2:

IMPALA-7030


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8299a8a97ad1f2bd1f2927e3111db8df1d3a3e5
Gerrit-Change-Number: 10381
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Tue, 15 May 2018 16:36:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6983: stress: don't write a null runtime profile

[Michael Brown (Code Review)]

Michael Brown has removed a vote on this change.

Change subject: IMPALA-6983: stress: don't write a null runtime profile
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/10381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic8299a8a97ad1f2bd1f2927e3111db8df1d3a3e5
Gerrit-Change-Number: 10381
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 

[Impala-ASF-CR] IMPALA-6995: avoid DCHECK in TimestampParse::Parse()

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-6995: avoid DCHECK in TimestampParse::Parse()
..


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10349/2//COMMIT_MSG@7
PS2, Line 7: avoid
> nit: Avoid
Different people just capitalise this differently if you look through the 
commit history


http://gerrit.cloudera.org:8080/#/c/10349/2/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

http://gerrit.cloudera.org:8080/#/c/10349/2/be/src/runtime/timestamp-parse-util.cc@88
PS2, Line 88: const int TimestampParser::DEFAULT_DATE_FMT_LEN;
> For my understanding, why are these needed? As far as I see these were used
They should have been here before but previous the generated code didn't need 
to reference the address of these variables. I think the use of min() changed 
the generated code enough that it caused problems (there was a linker error).


http://gerrit.cloudera.org:8080/#/c/10349/2/be/src/runtime/timestamp-parse-util.cc@372
PS2, Line 372: // when parsing fails.
> Please mention in the comment that 'd' and 't' is expected not to be NULL.
Done


http://gerrit.cloudera.org:8080/#/c/10349/2/be/src/runtime/timestamp-parse-util.cc@373
PS2, Line 373: static bool TimestampParseFailed(date* d, time_duration* t) {
> I find the return value of this function a bit weird. I see the intent to m
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a18ffd8893fe74f5830144300f745ce31477b1
Gerrit-Change-Number: 10349
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 16:08:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6995: avoid DCHECK in TimestampParse::Parse()

[Tim Armstrong (Code Review)]

Hello Gabor Kaszab, Kim Jin Chul,

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

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

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

Change subject: IMPALA-6995: avoid DCHECK in TimestampParse::Parse()
..

IMPALA-6995: avoid DCHECK in TimestampParse::Parse()

The bug was that the string's length is checked before trimming leading
and trailing spaces instead of afterwards. The bug has been present for
a long time but couldn't hit a DCHECK until recently.

Testing:
Added some backend tests that reproduce the crash.

Change-Id: I02a18ffd8893fe74f5830144300f745ce31477b1
---
M be/src/exprs/expr-test.cc
M be/src/runtime/timestamp-parse-util.cc
2 files changed, 56 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02a18ffd8893fe74f5830144300f745ce31477b1
Gerrit-Change-Number: 10349
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] Put ASAN options in CMakeLists.

[Tim Armstrong (Code Review)]

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

Change subject: Put ASAN options in CMakeLists.
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10404/1//COMMIT_MSG@10
PS1, Line 10: to avoid various failures.  In particular, backend tests fail
Maybe I'm missing something, but why does setting this in CMake help? I don't 
think we run these tests through cmake-generated output, but rather through 
shell scripts, at least in normal automated builds.

Do the environment variables set in a CMake script even carry over to processes 
launched by the CMake-generated build scripts? The CMake docs don't seem to be 
clear on the point.


http://gerrit.cloudera.org:8080/#/c/10404/1/be/CMakeLists.txt
File be/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/10404/1/be/CMakeLists.txt@154
PS1, Line 154:   set( ENV{ASAN_OPTIONS} "handle_segv=0 detect_leaks=0")
The other ASAN_OPTIONS settings also have allocator_may_return_null=1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3cbbd210c67750a48003f336bea1f3e1cb2d9e6b
Gerrit-Change-Number: 10404
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 15 May 2018 16:02:20 +
Gerrit-HasComments: Yes