[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Sat, 15 Sep 2018 04:03:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch inconsistent metadata

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

Change subject: IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch 
inconsistent metadata
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/687/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17389954780fa22d7866224c17e128458fffa545
Gerrit-Change-Number: 11403
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 15 Sep 2018 02:31:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch inconsistent metadata

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has uploaded a new patch set (#4) to the change originally 
created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/11403 )

Change subject: IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch 
inconsistent metadata
..

IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch inconsistent metadata

Given the granular catching in CatalogdMetaProvider, it's possible to
have a situation like the following:

- impalad has cached the table list including some table "t"
- some other daemon has issued deletion of "t"
- we try to access "t" before the invalidation has reached us via the
  statestore

In this case, we'll get an error back when we try to fetch the table
"t". This can confuse the planning process since it previously
determined that the table exists. Note that this may occur either when
"t" is first referenced or later during planning (eg when fetching a
specific partition post-pruning), so it wouldn't be possible to simply
convert it into a 'table does not exist' result.

The solution here is to throw InconsistentMetadataFetchException after
invalidating the table list and associated object. This then triggers a
re-plan of the query.

This patch implements such re-planning up to 10 times before eventually
giving up. Given that each attempt invalidates the inconsistent cache we
would normally expect it to succeed on the first such retry. The limit
of 10 retries is just to avoid infinite loops in the case of a bug.

Change-Id: I17389954780fa22d7866224c17e128458fffa545
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 192 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I17389954780fa22d7866224c17e128458fffa545
Gerrit-Change-Number: 11403
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch inconsistent metadata

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11403 )

Change subject: IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch 
inconsistent metadata
..


Patch Set 3:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2098
PS3, Line 2098: .
> Mention that appropriate LookupStats is set (error conditions)
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2125
PS3, Line 2125:
> spacing
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@312
PS3, Line 312: case DB_NOT_FOUND:
 : case FUNCTION_NOT_FOUND:
 : case TABLE_NOT_FOUND:
> indentation
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@317
PS3, Line 317: Could not load
> Instead say, it is not found? Could not load sounds like it is present but
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@322
PS3, Line 322:
> Preconditions lookup_stats == OK?
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@333
PS3, Line 333: (
> Can we include the information of the object that changed. Also mention tha
got it... made the message here, which is shown to users less low-level. But 
I've included these details in the log.


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

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@171
PS3, Line 171:
> Add some detail of why this is present?
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1073
PS3, Line 1073:  "
> Can we add "attempt xx of 10"...
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:

http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@158
PS3, Line 158: try:
> Add a docstring with an overview
Done


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@180
PS3, Line 180: replans_seen = [0]
> just curious, why is this a list?
seems to be a python scoping issue. the inner thread updates this so needs a 
mutable object to do so.
if there is a more idiomatic way to achieve this in python (and that works with 
required versions of python), let me know.
for now, added a comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17389954780fa22d7866224c17e128458fffa545
Gerrit-Change-Number: 11403
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 15 Sep 2018 01:38:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/686/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:45:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11447/3/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/11447/3/tests/conftest.py@55
PS3, Line 55:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/11447/3/tests/conftest.py@55
PS3, Line 55:
flake8: E251 unexpected spaces around keyword / parameter equals



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:43:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:30:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/685/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:30:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/684/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:21:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7573: Fix GroupingAggregator::Reset with output partition

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11446 )

Change subject: IMPALA-7573: Fix GroupingAggregator::Reset with 
output_partition_
..


Patch Set 1: Code-Review+2

(2 comments)

Couple of opportunities to improve comments but the fix makes sense, thanks for 
finding and fixing.

http://gerrit.cloudera.org:8080/#/c/11446/1/be/src/exec/grouping-aggregator.h
File be/src/exec/grouping-aggregator.h:

http://gerrit.cloudera.org:8080/#/c/11446/1/be/src/exec/grouping-aggregator.h@583
PS1, Line 583:   /// 'spilled_partitions_', and 'hash_partitions_' and then 
resets the lists,
Comment needs update to mention output_partition_.


http://gerrit.cloudera.org:8080/#/c/11446/1/be/src/exec/grouping-aggregator.cc
File be/src/exec/grouping-aggregator.cc:

http://gerrit.cloudera.org:8080/#/c/11446/1/be/src/exec/grouping-aggregator.cc@260
PS1, Line 260:   DCHECK(output_partition_ != nullptr);
Can you document this invariant (!output_iterator_.AtEnd() implies 
output_partition_ != nullptr) in the header?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6db8ec8479b18b5ed681d7ac438480711ab7a1ba
Gerrit-Change-Number: 11446
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:15:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 3:

Uploaded a quick v3 to prove concept, it seems to be a much better solution. 
The reason I'm looking for new solution is, the casting in the previous 
solution can fail for certain cases. That's the hole I was referring to. I will 
craft coding and probably add more tests later. One side note, this will change 
the behavior of "mod" for certain cases, thus may appear incompatible. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:14:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is 
DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it by makeing mod and % 
equivalent.
The solution is to replace "mod" with "%".

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 79 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 


[Impala-ASF-CR] IMPALA-7349: Add Admission control support for automatically setting per host memory limit for a query

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11157 )

Change subject: IMPALA-7349: Add Admission control support for automatically 
setting per host memory limit for a query
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11157/8/be/src/runtime/test-env.cc
File be/src/runtime/test-env.cc:

http://gerrit.cloudera.org:8080/#/c/11157/8/be/src/runtime/test-env.cc@116
PS8, Line 116:   int64_t mem_limit = query_options->__isset.mem_limit ? 
query_options->mem_limit : -1;
formatting is a bit weird - is this what clang-format did? If so, ok to leave.


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

http://gerrit.cloudera.org:8080/#/c/11157/7/be/src/scheduling/admission-controller.cc@1062
PS7, Line 1062: int64_t AdmissionController::ComputePerHostMemLimit(
  : const QuerySchedule& schedule, const TPoolConfig& pool_cfg, 
bool for_admission) {
  :   // If the min_query_mem_limit and max_query_mem_limit are not 
set in the pool config
  :   // then it falls back to traditional(old) behaviour, which 
means that, if for_admission
  :   // is false, it returns the mem_limit if it is set in the 
query options, else returns -1
  :   // which means no limit; if for_admission is true, it returns 
the mem_limit if it is set
  :   // in the query options, else returns the per host mem 
estimate calculated during
  :   // planning.
  :   bool mimic_old_behaviour =
  :   pool_cfg.min_query_mem_limit == 0 && 
pool_cfg.max_query_mem_limit == 0;
  :
  :   int64_t per_host_mem_limit = 0;
  :   bool has_query_option = false;
  :   const TQueryOptions& query_options = schedule.query_options();
  :   if (query_options.__isset.mem_limit && 
query_options.mem_limit > 0) {
  : per_host_mem_limit = query_options.mem_limit;
  : has_query_option = true;
  :   } else if (!for_admission && mimic_old_behaviour) {
  : return -1;
  :   }
  :
  :   if (!has_query_option) {
  : per_host_mem_limit = schedule.GetPerHostMemoryEstimate();
  : if (!mimic_old_behaviour) {
  :   int64_t min_mem_limit_required = 
ReservationUtil::GetMinMemLimitFromReservation(
  :   schedule.largest_min_reservation());
  :   per_host_mem_limit = max(per_host_mem_limit, 
min_mem_limit_required);
  : }
  :   }
  :
  :   if (!has_query_option || 
!pool_cfg.strict_min_max_query_mem_limit) {
  : DCHECK(pool_cfg.max_query_mem_limit <= 0
  : || pool_cfg.max_query_mem_limit >= 
pool_cfg.min_query_mem_limit);
  : if (pool_cfg.min_query_mem_limit > 0) {
  :   per_host_mem_limit = max(per_host_mem_limit, 
pool_cfg.min_query_mem_limit);
  : }
  : if (pool_cfg.max_query_mem_limit > 0) {
  :   per_host_mem_limit = min(per_host_mem_limit, 
pool_cfg.max_query_mem_limit);
  : }
  :   }
  :
  :   return per_host_mem_limit;
  : }
> how about I move this into QuerySchedule and do a schedule->UpdateMemREquir
I think that's ok, should just make sure we document the invariants and usage 
patterns in QuerySchedule.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifec00141651982f5975803c2165b7d7a10ebeaa6
Gerrit-Change-Number: 11157
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:08:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7519: Support elliptic curve ssl ciphers

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11376 )

Change subject: IMPALA-7519: Support elliptic curve ssl ciphers
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
Gerrit-Change-Number: 11376
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:04:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:02:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

2018-09-14 Thread Thomas Marshall (Code Review)
Hello Philip Zeyliger, David Knupp, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..

IMPALA-7576: Add a timeout for all E2E tests

We've been seeing a lot of hangs in tests lately. This can waste
test resources by keeping machines busy, cause loss of coverage when
subsequent tests don't run, and can be difficult to diagnose if its
not clear which test hung.

This patch introduces a timeout of 2 hours for normal builds and 4
hours for slow builds for all tests run under pytest by using the
pytest-timeout plugin.

The timeouts were chosen to be generous to avoid false positives.
In recent runs I examined, the longest running test is
test_decimal_fuzz, which took 63 minutes in a DEBUG build and
162 minutes in an ASAN build.

Testing:
- Ran locally with a reduced timeout and confirmed the test is
  timed out when expected.

Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
---
M infra/python/deps/stage2-requirements.txt
M tests/conftest.py
2 files changed, 6 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11447/1/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/11447/1/tests/run-tests.py@69
PS1, Line 69:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/11447/1/tests/run-tests.py@69
PS1, Line 69:
flake8: E251 unexpected spaces around keyword / parameter equals



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:49:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

2018-09-14 Thread Thomas Marshall (Code Review)
Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11447 )

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11447/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/11447/2/tests/run-tests.py@67
PS2, Line 67:   """ Custom pytest plugin to count the number of tests
> Can we make it a bit more obvious that this is a per-test setting, and that
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:48:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

2018-09-14 Thread Thomas Marshall (Code Review)
Hello Philip Zeyliger, David Knupp, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..

IMPALA-7576: Add a timeout for all E2E tests

We've been seeing a lot of hangs in tests lately. This can waste
test resources by keeping machines busy, cause loss of coverage when
subsequent tests don't run, and can be difficult to diagnose if its
not clear which test hung.

This patch introduces a timeout of 2 hours for normal builds and 4
hours for slow builds for all tests run under pytest by using the
pytest-timeout plugin.

The timeouts were chosen to be generous to avoid false positives.
In recent runs I examined, the longest running test is
test_decimal_fuzz, which took 63 minutes in a DEBUG build and
162 minutes in an ASAN build.

Testing:
- Ran locally with a reduced timeout and confirmed the test is
  timed out when expected.

Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
---
M infra/python/deps/stage2-requirements.txt
M tests/conftest.py
2 files changed, 6 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2:

I'm thinking about a different approach now, say, we can probably simply 
convert a "mod" to "%" when doing the parsing, this way, all code used by "%" 
would be shared, and thus making them really equivalent. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:37:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

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

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/683/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:32:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2:

> (2 comments)

Very good suggestion Vuk, I tried it out, and added some more tests and 
uncovered a hole in the solution. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:20:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

2018-09-14 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11447 )

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..


Patch Set 2: Code-Review+2

(1 comment)

Thanks for tackling this!

I'm +2 because it's a strict improvement over the current state. However, I 
think this might not cover custom cluster tests, because run-tests.py doesn't 
run them, and they're triggered by something else if I recall correctly. An 
alternative might be to shove this into conftest.py, or simply update 
tests/run-custom-cluster-tests.sh or bin/run-all-tests.sh.

(I'd also be happy if run-all-tests.py started running the custom cluster tests 
too, but that's definitely increasing the scope of this review unreasonably.)

http://gerrit.cloudera.org:8080/#/c/11447/2/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/11447/2/tests/run-tests.py@67
PS2, Line 67: # Timeout each tests after 2 hours for normal builds and 4 hours 
for slow builds
Can we make it a bit more obvious that this is a per-test setting, and that 
it's long because some tests (like test decimal fuzz under ASAN) do take hours. 
Because you used the word "build", I got confused whether this was the overall 
timeout or the per-test timeout.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:10:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7576: Add a timeout for all E2E tests

2018-09-14 Thread Thomas Marshall (Code Review)
Thomas Marshall has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/11447 )

Change subject: IMPALA-7576: Add a timeout for all E2E tests
..

IMPALA-7576: Add a timeout for all E2E tests

We've been seeing a lot of hangs in tests lately. This can waste
test resources by keeping machines busy, cause loss of coverage when
subsequent tests don't run, and can be difficult to diagnose if its
not clear which test hung.

This patch introduces a timeout of 2 hours for normal builds and 4
hours for slow builds for all tests run under pytest by using the
pytest-timeout plugin.

The timeouts were chosen to be generous to avoid false positives.
In recent runs I examined, the longest running test is
test_decimal_fuzz, which took 63 minutes in a DEBUG build and
162 minutes in an ASAN build.

Testing:
- Ran locally with a reduced timeout and confirmed the test is
  timed out when expected.

Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
---
M infra/python/deps/stage2-requirements.txt
M tests/run-tests.py
2 files changed, 9 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 


[Impala-ASF-CR] Add a timeout for all E2E tests

2018-09-14 Thread Thomas Marshall (Code Review)
Thomas Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11447


Change subject: Add a timeout for all E2E tests
..

Add a timeout for all E2E tests

We've been seeing a lot of hangs in tests lately. This can waste
test resources by keeping machines busy, cause loss of coverage when
subsequent tests don't run, and can be difficult to diagnose if its
not clear which test hung.

This patch introduces a timeout of 2 hours for normal builds and 4
hours for slow builds for all tests run under pytest by using the
pytest-timeout plugin.

The timeouts were chosen to be generous to avoid false positives.
In recent runs I examined, the longest running test is
test_decimal_fuzz, which took 63 minutes in a DEBUG build and
162 minutes in an ASAN build.

Testing:
- Ran locally with a reduced timeout and confirmed the test is
  timed out when expected.

Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
---
M infra/python/deps/stage2-requirements.txt
M tests/run-tests.py
2 files changed, 9 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I301dd27a9767bfaef2756282014ef457a31956bd
Gerrit-Change-Number: 11447
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7573: Fix GroupingAggregator::Reset with output partition

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

Change subject: IMPALA-7573: Fix GroupingAggregator::Reset with 
output_partition_
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/682/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6db8ec8479b18b5ed681d7ac438480711ab7a1ba
Gerrit-Change-Number: 11446
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 14 Sep 2018 22:18:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7573: Fix GroupingAggregator::Reset with output partition

2018-09-14 Thread Thomas Marshall (Code Review)
Thomas Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11446


Change subject: IMPALA-7573: Fix GroupingAggregator::Reset with 
output_partition_
..

IMPALA-7573: Fix GroupingAggregator::Reset with output_partition_

GroupingAggregator::Reset() doesn't call Close() on output_partition_,
which can lead to hitting a DCHECK(is_closed) in the Partition
destructor.

Testing:
- Added an e2e regression test.

Change-Id: I6db8ec8479b18b5ed681d7ac438480711ab7a1ba
---
M be/src/exec/grouping-aggregator.cc
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
2 files changed, 21 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6db8ec8479b18b5ed681d7ac438480711ab7a1ba
Gerrit-Change-Number: 11446
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 


[native-toolchain-CR] IMPALA-6772: Build ORC lib without libhdfspp

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11442 )

Change subject: IMPALA-6772: Build ORC lib without libhdfspp
..


Patch Set 1:

I just confirmed that this builds on the usual range of Linux platforms that we 
test.


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9
Gerrit-Change-Number: 11442
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 21:13:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/681/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:44:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:14:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 2:

Hi Dan and Vuk, thanks a lot for your review comments. I just uploaded a new 
rev to address some test failures. I will address your comments in next rev. 
Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Yongjun Zhang 
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:11:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is 
DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it for Decimal V2 mode. 
V1 behavior
is left as is since the V1 mode will be dropped soon per IMPALA-4924.

Testing:
Added unit tests to cover the above mentioned two cases for both Decimal V1 and 
V2 mode,
while the two cases are fixed in V2 mode, they remain different in V1 mode.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 72 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7519: Support elliptic curve ssl ciphers

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

Change subject: IMPALA-7519: Support elliptic curve ssl ciphers
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/680/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
Gerrit-Change-Number: 11376
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 19:32:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch inconsistent metadata

2018-09-14 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11403 )

Change subject: IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch 
inconsistent metadata
..


Patch Set 3:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2098
PS3, Line 2098: .
Mention that appropriate LookupStats is set (error conditions)


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2125
PS3, Line 2125:
spacing


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@312
PS3, Line 312: case DB_NOT_FOUND:
 : case FUNCTION_NOT_FOUND:
 : case TABLE_NOT_FOUND:
indentation


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@317
PS3, Line 317: Could not load
Instead say, it is not found? Could not load sounds like it is present but 
couldn't be loaded for somereason


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@322
PS3, Line 322:
Preconditions lookup_stats == OK?


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@333
PS3, Line 333: (
Can we include the information of the object that changed. Also mention that it 
changed? (The message here doesn't make much sense for those reading)


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

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@171
PS3, Line 171:
Add some detail of why this is present?


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1073
PS3, Line 1073:  "
Can we add "attempt xx of 10"...


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:

http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@158
PS3, Line 158: try:
Add a docstring with an overview


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@180
PS3, Line 180: replans_seen = [0]
just curious, why is this a list?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17389954780fa22d7866224c17e128458fffa545
Gerrit-Change-Number: 11403
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 19:02:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7519: Support elliptic curve ssl ciphers

2018-09-14 Thread Thomas Marshall (Code Review)
Hello Dan Burkert, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7519: Support elliptic curve ssl ciphers
..

IMPALA-7519: Support elliptic curve ssl ciphers

Thrift's SSLSocketFactory class does not support setting ciphers that
use ecdh. This patch modifies our existing subclass of
SSLSocketFactory to override the ciphers() method and enable ECDH.

The code for this was taken from be/src/kudu/security/tls_context.cc

Testing:
- Added a custom cluster test that verifies that a cluster with only
  ECDH ciphers enabled works.

Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
---
M be/src/rpc/thrift-server.cc
M tests/custom_cluster/test_client_ssl.py
2 files changed, 70 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
Gerrit-Change-Number: 11376
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/11443/1/fe/src/main/java/org/apache/impala/analysis/Expr.java@448
PS1, Line 448: equals("mod")
this seems fairly special cased. I'm wondering if the functionCall expr is the 
place to do this... it seems to have special cases there for v2 type handling.


http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@515
PS1, Line 515:  QUERY
pls add unit tests as well to AnalyzeExprsTest.java. if '%' and 'mod' are 
equiv, then it should not be hard to extend all tests against the '%' 
arithmetic expr to also confirm the same behavior with 'mod'.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 18:08:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism

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

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:

http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@132
PS2, Line 132: GetHdfsErrorMsg("Error reading from HDFS file: 
", *scan_range_->file_string()));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@142
PS2, Line 142:   position_in_file, 
*scan_range_->file_string(), GetHdfsErrorMsg("")));
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@207
PS2, Line 207: if (scan_range_->external_buffer_tag_ == 
ScanRange::ExternalBufferTag::CACHED_BUFFER) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@265
PS2, Line 265:   hadoopReadZero(exclusive_hdfs_fh_->file(), 
io_mgr_->cached_read_options(), scan_range_->len());
line too long (101 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 18:00:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@516
PS1, Line 516: # IMPALA-6202 - mod and % are not equivalent.
rather than talk about how the old code used to work, it'd be better to word 
this in the way the code should work: "MOD and % should be equivalent". The 
current wording could be interpreted to mean that they are currently not 
equivalent.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 17:41:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7351: Improve memory estimates for Hbase Scan Nodes

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11306 )

Change subject: IMPALA-7351: Improve memory estimates for Hbase Scan Nodes
..


Patch Set 2:

(6 comments)

I think this is pretty good, big improvement! Just some readability and 
maintainability suggestions.

http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java:

http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@534
PS2, Line 534: public
can this be package-private? Let's document that it's only exposed for testing, 
too.


http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@548
PS2, Line 548: isStatsMissing
This variable name sounds a bit ungrammatical to me. Maybe "statsAreMissing". 
Ok to ignore...


http://gerrit.cloudera.org:8080/#/c/11306/2/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/11306/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@630
PS2, Line 630: assertTrue
assertEquals() here and below. We should also consider doing a static import to 
avoid the repeated noise of "Assert."


http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@631
PS2, Line 631: BitUtil.roundUpToPowerOf2(Type.INT.getSlotSize()) * 2
I think I'd prefer just hardcoding the expected value instead of having 
slightly non-trivial logic here. Unless there's something I'm missing here that 
prevents that.

This comment applies below too.


http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@636
PS2, Line 636: columnList.clear();
Consider just constructing a new column list with 
https://google.github.io/guava/releases/23.0/api/docs/com/google/common/collect/Lists.html#newArrayList-E...-

Easier to understand the test if the variables are less stateful.


http://gerrit.cloudera.org:8080/#/c/11306/2/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@678
PS2, Line 678:   }
Also consider adding a test case with many columns, just for completeness. E.g. 
100 string columns or something like that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I583545c3f5e454854f111871c5fbc4f108ae4bff
Gerrit-Change-Number: 11306
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 17:16:53 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism

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

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc
File be/src/runtime/io/hdfs-file-reader.cc:

http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@132
PS1, Line 132: GetHdfsErrorMsg("Error reading from HDFS file: 
", *scan_range_->file_string()));
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@142
PS1, Line 142:   position_in_file, 
*scan_range_->file_string(), GetHdfsErrorMsg("")));
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@207
PS1, Line 207: if (scan_range_->external_buffer_tag_ == 
ScanRange::ExternalBufferTag::CACHED_BUFFER) {
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/11444/1/be/src/runtime/io/hdfs-file-reader.cc@265
PS1, Line 265:   hadoopReadZero(exclusive_hdfs_fh_->file(), 
io_mgr_->cached_read_options(), scan_range_->len());
line too long (101 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 17:10:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism

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

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
..


Patch Set 2:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/679/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 17:03:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism

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

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
..


Patch Set 1:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/678/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 16:52:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism

2018-09-14 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/11444 )

Change subject: IMPALA-7556: part 1: handle different file systems via 
polymorphism
..

IMPALA-7556: part 1: handle different file systems via polymorphism

This commit reorganizes some parts of the ScanRange class.

File operations are handled through the abstract FileReader
class. The interface supports positional read that will
be needed by IMPALA-5843. The concrete file operations are
implemented in sub-classes LocalFileReader and
HdfsFileReader.

File reader classes are responsible for setting counters and
metrics related to file operations.

The core logic haven't been changed significantly, but quite
a lot code fragments were relocated.

Testing: Debug exhaustive tests passed

Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
---
M be/src/runtime/io/CMakeLists.txt
M be/src/runtime/io/disk-io-mgr.h
A be/src/runtime/io/file-reader.cc
A be/src/runtime/io/file-reader.h
A be/src/runtime/io/hdfs-file-reader.cc
A be/src/runtime/io/hdfs-file-reader.h
A be/src/runtime/io/local-file-reader.cc
A be/src/runtime/io/local-file-reader.h
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
11 files changed, 765 insertions(+), 412 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7556: part1: handle different file systems via polymorphism

2018-09-14 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11444


Change subject: IMPALA-7556: part1: handle different file systems via 
polymorphism
..

IMPALA-7556: part1: handle different file systems via polymorphism

This commit reorganizes some parts of the ScanRange class.

File operations are handled through the abstract FileReader
class. The interface supports positional read that will
be needed by IMPALA-5843. The concrete file operations are
implemented in sub-classes LocalFileReader and
HdfsFileReader.

File reader classes are responsible for setting counters and
metrics related to file operations.

The core logic haven't been changed significantly, but quite
a lot code fragments were relocated.

Testing: Debug exhaustive tests passed

Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
---
M be/src/runtime/io/CMakeLists.txt
M be/src/runtime/io/disk-io-mgr.h
A be/src/runtime/io/file-reader.cc
A be/src/runtime/io/file-reader.h
A be/src/runtime/io/hdfs-file-reader.cc
A be/src/runtime/io/hdfs-file-reader.h
A be/src/runtime/io/local-file-reader.cc
A be/src/runtime/io/local-file-reader.h
M be/src/runtime/io/request-context.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
11 files changed, 765 insertions(+), 419 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82
Gerrit-Change-Number: 11444
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7519: Support elliptic curve ssl ciphers

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11376 )

Change subject: IMPALA-7519: Support elliptic curve ssl ciphers
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11376/2/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/11376/2/be/src/rpc/thrift-server.cc@380
PS2, Line 380:   throw TSSLException("failed to configure ECDH support");
> As far as I can tell from the openssl docs, none of the functions here popu
I'm not sure that I trust the OpenSSL docs to be that comprehensive. I ran into 
this with IMPALA-7018, which populated the error queue even though it's not 
explicit in the documentation.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1666ceabec51b425e8a82be1cf519e2ac35fa5a6
Gerrit-Change-Number: 11376
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 16:13:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7487: fix stress test handling of AC memory rejection

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11432 )

Change subject: IMPALA-7487: fix stress test handling of AC memory rejection
..


Patch Set 5:

Thanks David!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Gerrit-Change-Number: 11432
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 16:09:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7487: fix stress test handling of AC memory rejection

2018-09-14 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11432 )

Change subject: IMPALA-7487: fix stress test handling of AC memory rejection
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Gerrit-Change-Number: 11432
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 15:39:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7487: fix stress test handling of AC memory rejection

2018-09-14 Thread David Knupp (Code Review)
David Knupp has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11432 )

Change subject: IMPALA-7487: fix stress test handling of AC memory rejection
..

IMPALA-7487: fix stress test handling of AC memory rejection

The bug was introduced in IMPALA-7356 part 1, which started
classifying some memory errors as AC rejection errors. The
binary search has special handling of mem_limit_exceeded,
and ended up generating incorrect runtime info.

Fix the error classification by checking for the memory-related
error strings first, then rename mem_limit_exceeded to not_enough_memory
since it now encompasses a wide range of low memory conditions,
not just the "Memory limit exceeded" error.

I think the handling and classification of errors may need some
more thought, but this fix is sufficient to prevent the stress
test failures.

Testing:
Ran TPC-H stress test locally, which previous was reliably hitting the
error described in the JIRA.

Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Reviewed-on: http://gerrit.cloudera.org:8080/11432
Reviewed-by: David Knupp 
Tested-by: David Knupp 
---
M tests/stress/concurrent_select.py
1 file changed, 44 insertions(+), 17 deletions(-)

Approvals:
  David Knupp: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Gerrit-Change-Number: 11432
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7487: fix stress test handling of AC memory rejection

2018-09-14 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11432 )

Change subject: IMPALA-7487: fix stress test handling of AC memory rejection
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Gerrit-Change-Number: 11432
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 15:39:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7487: fix stress test handling of AC memory rejection

2018-09-14 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11432 )

Change subject: IMPALA-7487: fix stress test handling of AC memory rejection
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7526cc0bd49069712c1a1b13cbfb6453f6afa46
Gerrit-Change-Number: 11432
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 15:38:52 +
Gerrit-HasComments: No


[native-toolchain-CR] IMPALA-6772: Build ORC lib without libhdfspp

2018-09-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11442 )

Change subject: IMPALA-6772: Build ORC lib without libhdfspp
..


Patch Set 1:

(1 comment)

I think this makes sense to me.

http://gerrit.cloudera.org:8080/#/c/11442/1/build.sh
File build.sh:

http://gerrit.cloudera.org:8080/#/c/11442/1/build.sh@37
PS1, Line 37:   if [ $PACKAGE == 'ORC' ]; then
Can you revert this part? I agree the ergonomics of build.sh are bad - you need 
to run "LZ4_VERSION=... PROTOBUF_VERSION=...  ./build.sh orc ...". I don't 
think this is the right solution though.

Maybe we need some metadata somewhere that tracks the mapping between component 
versions.

The bad thing about native-toolchain is that we're kind of implementing a 
custom build system in bash. Which is ... suboptimal.



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9
Gerrit-Change-Number: 11442
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 15:19:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 1: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 11:40:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/677/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Sep 2018 08:33:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

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

Change subject: WIP IMPALA-7527: add fetch-from-catalogd cache info to profile
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/675/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Gerrit-Change-Number: 11388
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 08:02:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 14 Sep 2018 07:59:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

2018-09-14 Thread Yongjun Zhang (Code Review)
Yongjun Zhang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11443


Change subject: IMPALA-6202. mod and % are not equivalent.
..

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is 
DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it for Decimal V2 mode. 
V1 behavior
is left as is since the V1 mode will be dropped soon per IMPALA-4924.

Testing:
Added unit tests to cover the above mentioned two cases for both Decimal V1 and 
V2 mode,
while the two cases are fixed in V2 mode, they remain different in V1 mode.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 75 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang 


[Impala-ASF-CR] WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

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

Change subject: WIP IMPALA-7527: add fetch-from-catalogd cache info to profile
..


Patch Set 5:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/676/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Gerrit-Change-Number: 11388
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 07:50:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has uploaded a new patch set (#5) to the change originally 
created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/11388 )

Change subject: WIP IMPALA-7527: add fetch-from-catalogd cache info to profile
..

WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

This patch adds a Java wrapper for a RuntimeProfile object. The wrapper
supports some basic operations like non-hierarchical counters and
informational strings.

During planning, a profile is created, and passed back to the backend as
part of the ExecRequest. The backend then updates the query profile
based on the info emitted from the frontend.

This patch also adds the first use case for this profile information:
the CatalogdMetaProvider emits counters for cache hits, misses, and
fetch times, broken down by metadata category.

The emitted profile is a bit of a superset of the existing 'timeline'
functionality. However, it seems that some tools may parse the timeline
in its current location in the profile, so moving it might be
incompatible. I elected to leave that alone for now and just emit
counters in the new profile.

WIP for a few reasons:
- needs tests
- maybe the amount of information is too verbose to include in every
  profile? Should we only include such info on misses, since we assume
  hits don't contribute to the timing much? Should we not bother
  breaking down by category?
- worth trying to include the byte sizes of metadata fetched?

Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M be/src/util/runtime-profile.h
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
A fe/src/main/java/org/apache/impala/service/FrontendProfile.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
9 files changed, 297 insertions(+), 49 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Gerrit-Change-Number: 11388
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has uploaded a new patch set (#4) to the change originally 
created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/11388 )

Change subject: WIP IMPALA-7527: add fetch-from-catalogd cache info to profile
..

WIP IMPALA-7527: add fetch-from-catalogd cache info to profile

This patch adds a Java wrapper for a RuntimeProfile object. The wrapper
supports some basic operations like non-hierarchical counters and
informational strings.

During planning, a profile is created, and passed back to the backend as
part of the ExecRequest. The backend then updates the query profile
based on the info emitted from the frontend.

This patch also adds the first use case for this profile information:
the CatalogdMetaProvider emits counters for cache hits, misses, and
fetch times, broken down by metadata category.

The emitted profile is a bit of a superset of the existing 'timeline'
functionality. However, it seems that some tools may parse the timeline
in its current location in the profile, so moving it might be
incompatible. I elected to leave that alone for now and just emit
counters in the new profile.

WIP for a few reasons:
- needs tests
- maybe the amount of information is too verbose to include in every
  profile? Should we only include such info on misses, since we assume
  hits don't contribute to the timing much? Should we not bother
  breaking down by category?
- worth trying to include the byte sizes of metadata fetched?

Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M be/src/util/runtime-profile.h
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
A fe/src/main/java/org/apache/impala/service/FrontendProfile.java
M fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
9 files changed, 294 insertions(+), 49 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Gerrit-Change-Number: 11388
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] WIP: IMPALA-7527: add fetch-from-catalogd cache info to profile

2018-09-14 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11388 )

Change subject: WIP: IMPALA-7527: add fetch-from-catalogd cache info to profile
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@369
PS3, Line 369: addStatsToProfile
> Yea, CacheStats will give top-level stats, but not per-object-type stats, e
agreed, lets get the information out, then work on making it easier.


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

http://gerrit.cloudera.org:8080/#/c/11388/3/fe/src/main/java/org/apache/impala/service/FrontendProfile.java@73
PS3, Line 73:* Create a new profile, setting it as the current thread-lcoal 
profile for the
> spelling: thread-local
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I808d55a225338912ebaebd0cf71c36db944b7276
Gerrit-Change-Number: 11388
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 07:27:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

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

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 42: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 42
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 06:03:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

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

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Reviewed-on: http://gerrit.cloudera.org:8080/11314
Reviewed-by: Vuk Ercegovac 
Tested-by: Impala Public Jenkins 
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
34 files changed, 1,365 insertions(+), 154 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 43
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac