[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-07 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13992


Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..

IMPALA-8791: Handle the case where there is no fragment scheduled on
the coordinator

This patch fixes a bug where if an insert or CTAS query has no
fragments scheduled on the coordinator and a mem limit is to be
enforced on the query (either through query option or automatically
through estimates) then the same limit is also applied to the
coordinator backend even though it does not execute anything.

Highlights:
- coord_backend_mem_to_admit_/mem_limit will always refer to the memory
to admit/limit for the coordinator regardless of which fragments are
scheduled on it.

- There will always be a BackendExecParams added for the coordinator
because coordinator always spawns a QueryState object with a mem_tracker
for tracking runtime filter mem and the result set cache. For the case
where this BackendExecParams is empty (no instances scheduled) it would
ensure that some minimal amount of memory is accounted for by the
admission controller and the right mem limit is applied to the
QueryState spawned by the coordinator

- added changes to Coordinator and Coordinator::BackendState classes
to handle an empty BackendExecParams object

Testing:
The following cases need to be tested where the kind of fragments
schduled on the coordinator backend are:
1. Coordinator fragment + other exec fragments
2. Coordinator fragment only
3. other exec fragments only (eg. insert into values OR insert
   into select 1)
4. No fragments, but coordinator still creates a QueryState

Case 1 is covered by tests working with non-dedicated coordinators.
Rest are covered by test_mem_limit_dedicated_coordinator in
test_admission_controller.py

Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-query/queries/QueryTest/dedicated-coord-mem-estimates.test
12 files changed, 151 insertions(+), 120 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4178/ : 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/13992
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:22:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/admission-controller.cc@442
PS2, Line 442:   && CanMemLimitAccommodateReservation(
Nice to remove this special case.


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

http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.h@81
PS2, Line 81:   // Indicates whether this backend will run on the coordinator 
backend.
nit: wording is a bit weird. Maybe just "Indicates whether this backend is the 
coordinator".


http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.h@374
PS2, Line 374: requiresCoordinatorFragment
nit: RequiresCoordinatorFragment()


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

http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.cc@261
PS2, Line 261: only_coord_fragment_schduled
scheduled?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Aug 2019 23:45:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-08 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.h@81
PS2, Line 81:   // Indicates whether this backend will run on the coordinator 
backend.
> nit: wording is a bit weird. Maybe just "Indicates whether this backend is
Done


http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.h@374
PS2, Line 374: requiresCoordinatorFragment
> nit: RequiresCoordinatorFragment()
Done


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

http://gerrit.cloudera.org:8080/#/c/13992/2/be/src/scheduling/query-schedule.cc@261
PS2, Line 261: only_coord_fragment_schduled
> scheduled?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Aug 2019 18:31:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-08 Thread Bikramjeet Vig (Code Review)
Hello Andrew Sherman, Lars Volker, Abhishek Rawat, Tim Armstrong, Impala Public 
Jenkins,

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

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

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

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..

IMPALA-8791: Handle the case where there is no fragment scheduled on
the coordinator

This patch fixes a bug where if an insert or CTAS query has no
fragments scheduled on the coordinator and a mem limit is to be
enforced on the query (either through query option or automatically
through estimates) then the same limit is also applied to the
coordinator backend even though it does not execute anything.

Highlights:
- coord_backend_mem_to_admit_/mem_limit will always refer to the memory
to admit/limit for the coordinator regardless of which fragments are
scheduled on it.

- There will always be a BackendExecParams added for the coordinator
because coordinator always spawns a QueryState object with a mem_tracker
for tracking runtime filter mem and the result set cache. For the case
where this BackendExecParams is empty (no instances scheduled) it would
ensure that some minimal amount of memory is accounted for by the
admission controller and the right mem limit is applied to the
QueryState spawned by the coordinator

- added changes to Coordinator and Coordinator::BackendState classes
to handle an empty BackendExecParams object

Testing:
The following cases need to be tested where the kind of fragments
schduled on the coordinator backend are:
1. Coordinator fragment + other exec fragments
2. Coordinator fragment only
3. other exec fragments only (eg. insert into values OR insert
   into select 1)
4. No fragments, but coordinator still creates a QueryState

Case 1 is covered by tests working with non-dedicated coordinators.
Rest are covered by test_mem_limit_dedicated_coordinator in
test_admission_controller.py

Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-query/queries/QueryTest/dedicated-coord-mem-estimates.test
12 files changed, 154 insertions(+), 123 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-08 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 3:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Aug 2019 18:50:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-08 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 08 Aug 2019 18:51:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 12 Aug 2019 17:47:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 12 Aug 2019 17:47:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 12 Aug 2019 21:55:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/13992 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..

IMPALA-8791: Handle the case where there is no fragment scheduled on
the coordinator

This patch fixes a bug where if an insert or CTAS query has no
fragments scheduled on the coordinator and a mem limit is to be
enforced on the query (either through query option or automatically
through estimates) then the same limit is also applied to the
coordinator backend even though it does not execute anything.

Highlights:
- coord_backend_mem_to_admit_/mem_limit will always refer to the memory
to admit/limit for the coordinator regardless of which fragments are
scheduled on it.

- There will always be a BackendExecParams added for the coordinator
because coordinator always spawns a QueryState object with a mem_tracker
for tracking runtime filter mem and the result set cache. For the case
where this BackendExecParams is empty (no instances scheduled) it would
ensure that some minimal amount of memory is accounted for by the
admission controller and the right mem limit is applied to the
QueryState spawned by the coordinator

- added changes to Coordinator and Coordinator::BackendState classes
to handle an empty BackendExecParams object

Testing:
The following cases need to be tested where the kind of fragments
schduled on the coordinator backend are:
1. Coordinator fragment + other exec fragments
2. Coordinator fragment only
3. other exec fragments only (eg. insert into values OR insert
   into select 1)
4. No fragments, but coordinator still creates a QueryState

Case 1 is covered by tests working with non-dedicated coordinators.
Rest are covered by test_mem_limit_dedicated_coordinator in
test_admission_controller.py

Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Reviewed-on: http://gerrit.cloudera.org:8080/13992
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-query/queries/QueryTest/dedicated-coord-mem-estimates.test
12 files changed, 154 insertions(+), 123 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If5631fa1490d9612ffac3c4c4715348de47d6df2
Gerrit-Change-Number: 13992
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/14058


Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..

IMPALA-8791: Handle the case where there is no fragment scheduled on
the coordinator

This patch fixes a bug where if an insert or CTAS query has no
fragments scheduled on the coordinator and a mem limit is to be
enforced on the query (either through query option or automatically
through estimates) then the same limit is also applied to the
coordinator backend even though it does not execute anything.

Highlights:
- coord_backend_mem_to_admit_/mem_limit will always refer to the memory
to admit/limit for the coordinator regardless of which fragments are
scheduled on it.

- There will always be a BackendExecParams added for the coordinator
because coordinator always spawns a QueryState object with a mem_tracker
for tracking runtime filter mem and the result set cache. For the case
where this BackendExecParams is empty (no instances scheduled) it would
ensure that some minimal amount of memory is accounted for by the
admission controller and the right mem limit is applied to the
QueryState spawned by the coordinator

- added changes to Coordinator and Coordinator::BackendState classes
to handle an empty BackendExecParams object

Testing:
The following cases need to be tested where the kind of fragments
schduled on the coordinator backend are:
1. Coordinator fragment + other exec fragments
2. Coordinator fragment only
3. other exec fragments only (eg. insert into values OR insert
   into select 1)
4. No fragments, but coordinator still creates a QueryState

Case 1 is covered by tests working with non-dedicated coordinators.
Rest are covered by test_dedicated_coordinator_planner_estimates and
test_sanity_checks_dedicated_coordinator in
test_admission_controller.py

Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-query/queries/QueryTest/dedicated-coord-mem-estimates.test
M tests/custom_cluster/test_admission_controller.py
13 files changed, 167 insertions(+), 125 deletions(-)



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

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


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14058/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/14058/1/tests/custom_cluster/test_admission_controller.py@551
PS1, Line 551: # Make sure query execution works perfectly for a query that 
does not have any
 : # fragments schdeuled on the coordinator, but has 
runtime-filters that need to be
 : # aggregated at the coordinator.
removed the DCHECK in Coordinator::BackendState::PublishFilter and added this 
test that have triggered that dcheck.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:01:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4239/ : 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/14058
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:39:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 13 Aug 2019 23:58:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:53:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:53:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..

IMPALA-8791: Handle the case where there is no fragment scheduled on
the coordinator

This patch fixes a bug where if an insert or CTAS query has no
fragments scheduled on the coordinator and a mem limit is to be
enforced on the query (either through query option or automatically
through estimates) then the same limit is also applied to the
coordinator backend even though it does not execute anything.

Highlights:
- coord_backend_mem_to_admit_/mem_limit will always refer to the memory
to admit/limit for the coordinator regardless of which fragments are
scheduled on it.

- There will always be a BackendExecParams added for the coordinator
because coordinator always spawns a QueryState object with a mem_tracker
for tracking runtime filter mem and the result set cache. For the case
where this BackendExecParams is empty (no instances scheduled) it would
ensure that some minimal amount of memory is accounted for by the
admission controller and the right mem limit is applied to the
QueryState spawned by the coordinator

- added changes to Coordinator and Coordinator::BackendState classes
to handle an empty BackendExecParams object

Testing:
The following cases need to be tested where the kind of fragments
schduled on the coordinator backend are:
1. Coordinator fragment + other exec fragments
2. Coordinator fragment only
3. other exec fragments only (eg. insert into values OR insert
   into select 1)
4. No fragments, but coordinator still creates a QueryState

Case 1 is covered by tests working with non-dedicated coordinators.
Rest are covered by test_dedicated_coordinator_planner_estimates and
test_sanity_checks_dedicated_coordinator in
test_admission_controller.py

Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Reviewed-on: http://gerrit.cloudera.org:8080/14058
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-query/queries/QueryTest/dedicated-coord-mem-estimates.test
M tests/custom_cluster/test_admission_controller.py
13 files changed, 167 insertions(+), 125 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8791: Handle the case where there is no fragment scheduled on the coordinator

2019-08-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14058 )

Change subject: IMPALA-8791: Handle the case where there is no fragment 
scheduled on the coordinator
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4fba02bb7b20553a20634f8c5989d43ba2c0721
Gerrit-Change-Number: 14058
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 14 Aug 2019 06:11:44 +
Gerrit-HasComments: No