You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2019/09/05 21:56:17 UTC

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

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


Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group that only contains the local coordinator to which the
query was submitted. This allows running coordinator only queries
regardless of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
7 files changed, 55 insertions(+), 27 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/admission-controller.h@973
PS5, Line 973:   /// the query is a coordinator only query, then a reserved empty group is returned
nit: punctuation


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.h@72
PS5, Line 72:   /// ranges in the query exec request.
Mention here that executor_config must contain a non-empty group unless IsCoordinatorOnlyQuery() is true?


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc@143
PS5, Line 143:       bool exec_at_coord = (fragment.partition.type == TPartitionType::UNPARTITIONED);
Can we add a DCHECK to verify that exec_at_coord is true if executor_config only contains the empty group? It's probably implicitly checked by the assignment computation, but I think it would be good to tie this together with IsCoordinatorQuery().


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc@466
PS5, Line 466:   // case.
I think here it would help future-us if we include the purpose of this group, e.g similar to this: 

> This group is used to schedule fragments where the caller of this method has determined that they need to be scheduled on the coordinator only. Note that this includes queries where the whole query should run on the coordinator, as is determined by Scheduler::IsCoordinatorOnlyQuery(). For those queries, the AdmissionController will pass an empty executor group and rely on this method being called with exec_at_coord = true.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:03:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 2:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2019 20:51:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 9: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 9
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 02:27:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 3:

(7 comments)

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

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@625
PS2, Line 625:     string* rejection_reason) {
> nit: drop std:: in .cc files
Done


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@1122
PS2, Line 1122:         || ClusterMembershipMgr::GetEmptyExecutorGroup() == executor_group);
> Would this group ever be unhealthy?
the CoordintorOnlyExecutorGroup is empty so its always unhealthy technically. The only way for this to be actually unhealthy is for the coordinator (which is running this query goes down and thats is impossible)


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@1127
PS2, Line 1127:     VLOG(3) << "Scheduling for executor group: " << group_name << " with "
> I wonder if this log statement will be confusing when scheduling on the coo
I think if we communicate that the group named "coordinator-only-group" is a special group, it should be ok, since we'll be in the same boat when we look at the query profile and it prints out the "Executor Group" as "coordinator-only-group".
One way to make sure that this name is special is by reserving this name and not allowing any exec groups to have this listed as their group name, either by rejecting it at startup or just ignoring it.


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h@177
PS2, Line 177:   /// Returns a pointer to the static empty group reserved for scheduling coord only
> line too long (93 > 90)
Done


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h@220
PS2, Line 220: 
> I understand why it is like this, but I think it's a problem that the name 
I agree. Done.


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.cc@48
PS2, Line 48: static const string EMPTY_GROUP_NAME("empty group (using coordinator only)");
> This could be "empty group (using coordinator)" (or just the empty string)?
"empty group (using coordinator)" => this seems good to me as the empty one can be confusing. I dont really have a preference here, other than having a name that communicates implicitly that it means that the query will only run on the coordinator, and your suggestion captures this pretty perfectly.


http://gerrit.cloudera.org:8080/#/c/14183/2/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14183/2/tests/custom_cluster/test_executor_groups.py@97
PS2, Line 97: expected_group = "E
> nit: expected_group ?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 00:30:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 6:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/14183/6/be/src/scheduling/admission-controller.cc@a737
PS6, Line 737: 
Looks like this reason can't be returned any more. Was this deliberate?

Is the idea that these queries will just get queued? Or that they'll fail a different check?


http://gerrit.cloudera.org:8080/#/c/14183/6/be/src/scheduling/admission-controller.cc@a745
PS6, Line 745: 
Same with this one?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 10 Oct 2019 22:08:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Reviewed-on: http://gerrit.cloudera.org:8080/14183
Reviewed-by: Bikramjeet Vig <bi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 110 insertions(+), 52 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 12
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 22:37:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 11
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Jun 2020 01:35:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 11:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 11
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 20:59:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 4:

rebased


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 01:05:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
8 files changed, 94 insertions(+), 75 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h@212
PS7, Line 212:   const ExecutorGroup empty_exec_group_;
> I don't feel too strongly about this, but it might be better in some ways t
Done


http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc@558
PS7, Line 558:   VLOG_ROW << "Exec at coord is " << (exec_at_coord ? "true" : "false");
> While we're here, can we make this VLOG(2) or equivalent? It is kinda spamm
Done. Thanks for catching this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 20:06:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
9 files changed, 106 insertions(+), 104 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 2:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@625
PS2, Line 625:     std::string* rejection_reason) {
nit: drop std:: in .cc files


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@1122
PS2, Line 1122:         || ClusterMembershipMgr::GetCoordintorOnlyExecutorGroup() == executor_group);
Would this group ever be unhealthy?


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@1127
PS2, Line 1127:     VLOG(3) << "Scheduling for executor group: " << group_name << " with "
I wonder if this log statement will be confusing when scheduling on the coord-only group


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h@220
PS2, Line 220: coord_only_exec_group_
I understand why it is like this, but I think it's a problem that the name does not reflect that it's empty. If you see "coord_only_exec_group_" you pretty much expect that it only contains the coordinator. I think it'd be better to call it "empty_exec_group" and explain why it is like that in places where we use it.


http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.cc@48
PS2, Line 48: static const string COORD_ONLY_GROUP_NAME("coordinator-only-group");
This could be "empty group (using coordinator)" (or just the empty string)?


http://gerrit.cloudera.org:8080/#/c/14183/2/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14183/2/tests/custom_cluster/test_executor_groups.py@97
PS2, Line 97: expected_in_profile
nit: expected_group ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 23 Sep 2019 20:32:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
8 files changed, 95 insertions(+), 76 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 2:

(1 comment)

As discussed offline, added a pseudo empty exec groups that we use only for scheduling coord only queries. I skipped making changes to the AssignmentCtx since it was turning out to be a bit unwieldy. We can take it up as clean up in the future.

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

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/admission-controller.cc@a690
PS2, Line 690: 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
             : 
like we discussed offline, got rid of these checks that dont make sense for groups.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2019 20:16:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 4:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 01:34:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 11
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 20:34:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
7 files changed, 99 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/14183/8
-- 
To view, visit http://gerrit.cloudera.org:8080/14183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 7:

(2 comments)

This looks good overall, had minor comments only.

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h@212
PS7, Line 212:   static const ExecutorGroup empty_exec_group_;
I don't feel too strongly about this, but it might be better in some ways to have this be a field of ClusterMembershipMgr, so that we don't run the static destructors, etc when the process shuts down.


http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc@558
PS7, Line 558:   VLOG_QUERY << "Exec at coord is " << (exec_at_coord ? "true" : "false");
While we're here, can we make this VLOG(2) or equivalent? It is kinda spammy when I look at logs.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jun 2020 00:47:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 9
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 21:16:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/admission-controller.h@973
PS5, Line 973:   /// the query is a coordinator only query then a reserved empty group is returned
> nit: punctuation
Done


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.h@72
PS5, Line 72:   /// ranges in the query exec request. 'executor_config' must contain a non-empty group
> Mention here that executor_config must contain a non-empty group unless IsC
Done


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc@143
PS5, Line 143:       bool exec_at_coord = (fragment.partition.type == TPartitionType::UNPARTITIONED);
> Can we add a DCHECK to verify that exec_at_coord is true if executor_config
Done


http://gerrit.cloudera.org:8080/#/c/14183/5/be/src/scheduling/scheduler.cc@466
PS5, Line 466:   // used to schedule scan ranges for the plan node passed where the caller of this method
> I think here it would help future-us if we include the purpose of this grou
Done. Slightly modified your suggested description to mention scan ranges being assigned for plan nodes



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Oct 2019 00:26:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jun 2020 00:07:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 20:35:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 6: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Oct 2019 00:33:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 1:

(5 comments)

I think it would be good to unify the ways in which we pass the local BE desc into the scheduler. With this change we pass it both as a BE desc and as the coordinator-only group. We could stay with the old approach and pass an ExecutorConfig with an empty executor group, and then build the coordinator-only group as we did before in the scheduler. I don't see an easy way to pass it with a coordinator-only group when also using a regular executor group for scheduling.

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h@93
PS1, Line 93:     ExecutorGroup local_coord_only_group = ExecutorGroup("coordinator-only-group");
It seems that so far we have created a group with the coordinator on-demand in the scheduler. Having a special group inside executor_groups could be a cleaner approach. Can we move this group there?


http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@143
PS1, Line 143:       bool exec_at_coord = (fragment.partition.type == TPartitionType::UNPARTITIONED);
Should exec_at_coord now be replaced with IsCoordinatorOnlyQuery()?


http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@465
PS1, Line 465:   ExecutorGroup coord_only_executor_group("temp-coordinator-only-group");
This should now use the coordinator-only group, no?


http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: schduled
nit: typo


http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: ideally
Why "ideally" instead of "that gets scheduled".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 22:54:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 110 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/14183/11
-- 
To view, visit http://gerrit.cloudera.org:8080/14183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 11
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 9
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 21:16:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 1:

(5 comments)

> (5 comments)
 > 
 > I think it would be good to unify the ways in which we pass the
 > local BE desc into the scheduler. With this change we pass it both
 > as a BE desc and as the coordinator-only group. We could stay with
 > the old approach and pass an ExecutorConfig with an empty executor
 > group, and then build the coordinator-only group as we did before
 > in the scheduler. I don't see an easy way to pass it with a
 > coordinator-only group when also using a regular executor group for
 > scheduling.

We cant pass an executor group because further down the code path we expect an exec group associated with an admitted schedule to have non-zero executors as we use the group size for making admission decisions.

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h@93
PS1, Line 93:     ExecutorGroup local_coord_only_group = ExecutorGroup("coordinator-only-group");
> It seems that so far we have created a group with the coordinator on-demand
I thought of that but decided against it since the whole premise of an executor group is that it is composed of executors only, which is why i called this a "pseudo group". Also adding a special group in the ExecutorGroups list means there will always be a healthy group with ""coordinator-only-group"" being a reserved group name (not sure how we can enforce that). moreover, this group wont be bound by the group_name->resource_pool mapping rule. Basically a bunch of special handling will be required for that special exec group and having that implicit in a regular vector of ExecutorGroup objects would be weird.

I initially thought of creating ExecutorGroups class that encompasses these special cases, and abstract away some of that logic there but if I keep going that route I would end up with something like the all encompassing Snapshot struct, so I just added the pseudo group to it.

I agree all this is a bit messy so open to suggestions. Maybe we can just pass the full snapshot and a list of exec groups for the resource pool to the scheduler and handle some of the messiness there (handling the IsCoordinatorOnlyQuery, temp-coordinator-only-group parts, Scheduler::ExecutorConfig parts).


http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@143
PS1, Line 143:       bool exec_at_coord = (fragment.partition.type == TPartitionType::UNPARTITIONED);
> Should exec_at_coord now be replaced with IsCoordinatorOnlyQuery()?
the exec_at_coord used here is at the fragment level to identify if a fragment is to be scheduled on the coordinator, whereas IsCoordinatorOnlyQuery is for the whole query that also makes sure the plan contains only one fragment.


http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@465
PS1, Line 465:   ExecutorGroup coord_only_executor_group("temp-coordinator-only-group");
> This should now use the coordinator-only group, no?
Unfortunately, the way scheduling works, this temp group is required because ComputeScanRangeAssignment is done for all queries and "non-coordinator only" queries would not have coordinator-only-group in their ExecutorConfig


http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:

http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: schduled
> nit: typo
will fix in the next patch


http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: ideally
> Why "ideally" instead of "that gets scheduled".
will fix in the next patch



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 23:45:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/2/be/src/scheduling/cluster-membership-mgr.h@177
PS2, Line 177:   /// Returns a pointer to the static empty group reserved for scheduling coord only queries.
line too long (93 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2019 20:12:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
9 files changed, 97 insertions(+), 103 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 6
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Oct 2019 19:36:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Jun 2020 20:52:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 7:

Rebased on top of IMPALA-9077


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jun 2020 23:23:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 5
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 01:07:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "coordinator-only-group" which is empty. This
allows running coordinator only queries regardless of the presence
of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/executor-group.cc
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
8 files changed, 95 insertions(+), 76 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 3:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2019 01:09:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................

IMPALA-8830: Fix executor group assignment of coordinator only queries

With this fix, coordinator only queries are submitted to a pseudo
executor group named "empty group (using coordinator only)" which
is empty. This allows running coordinator only queries regardless
of the presence of any healthy executor groups.

Testing:
Added a custom cluster test and modified tests that relied on
coordinator only queries to be queued in absence of executor groups.

Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M tests/custom_cluster/test_executor_groups.py
7 files changed, 98 insertions(+), 45 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries
......................................................................


Patch Set 11: Code-Review+2

Fixed the failed test, test_dedicated_coordinator_without_executors.
Carrying over Tim's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 11
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jun 2020 20:34:10 +0000
Gerrit-HasComments: No