You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2019/11/20 21:02:01 UTC

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot 

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


Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................

IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

This change improves the cluster membership snapshot we maintain in the
frontend in cases where all executors have been shut down or none have
started yet.

Prior to this change when configuring Impala with executor groups, the
planner might see a ExecutorMembershipSnapshot that has no executors in
it. This could happen if the first executor group had not started up
yet, or if all executor groups had been shutdown. If this happened, the
planner would make sub-optimal decisions, e.g. decide on a broadcast
join vs a PHJ.

With this change if no executors have been registered so far, the
planner will use a configurable default which should be set to the
expected number of executors. After executors come online, the planner
will use the size of the largest healthy executor group, and it will
hold on to the group's size even if it shuts down or becomes unhealthy.
This allows the planner to work on the assumption that a healthy
executor group of the same size will eventually come online to execute
the query.

Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
---
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
A fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 251 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

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

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

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................

IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

This change improves the cluster membership snapshot we maintain in the
frontend in cases where all executors have been shut down or none have
started yet.

Prior to this change when configuring Impala with executor groups, the
planner might see a ExecutorMembershipSnapshot that has no executors in
it. This could happen if the first executor group had not started up
yet, or if all executor groups had been shutdown. If this happened, the
planner would make sub-optimal decisions, e.g. decide on a broadcast
join vs a partitioned hash join.

With this change if no executors have been registered so far, the
planner will use the expected number of executors which can be set using
the -num_expected_executors flag and is 20 by default. After executors
come online, the planner will use the size of the largest healthy
executor group, and it will hold on to the group's size even if it shuts
down or becomes unhealthy. This allows the planner to work on the
assumption that a healthy executor group of the same size will
eventually come online to execute the query.

Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
---
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
A fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 266 insertions(+), 18 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Wed, 27 Nov 2019 23:25:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Tue, 03 Dec 2019 20:11:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 1: Code-Review+2

(5 comments)

LGTM I have a few nits

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

http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@18
PS1, Line 18: join vs a PHJ.
Maybe spell out "partitioned hash join" (if that is what this is)?
Update: I see this in a few places, as a newbie I like seeing things spelled out.


http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@21
PS1, Line 21: planner will use a configurable default which should be set to the
Maybe mention that the default is 20?


http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc@253
PS1, Line 253: DEFINE_int32(num_expected_executors, 20, "The number of executors that the planner "
Maybe 'the number of executors that are expected to be available for the execution of a single query'


http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java:

http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@39
PS1, Line 39:   // The set of hosts that are members of the cluster given by hostname.
Should we add some caveats here (and for ipAddresses_) that these values are only present if we are not using executor groups (IIUC)


http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java
File fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java:

http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java@83
PS1, Line 83:     // Adding two or more executors will make the planner switch to a PHJ.
partitioned hash join



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 00:34:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14756/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/14756/2/be/src/runtime/exec-env.cc@198
PS2, Line 198:   if (!group) return;
I think this lack of updating needs some explanation - the implications are a bit subtle. Does this mean that we stop sending updates to the frontend if the number of executors decreases? I guess the idea is that the frontend uses the last snapshot that had a healthy executor group in it?

I actually might've thought that we'd send an update to the frontend that explicitly let it know there were no healthy groups.

After reading through this, I think I understand the intent. I wonder if the logic might be clearer if we sent the update to the frontend anyway and let it ignore the change, just so that this key bit of the logic is in ExecutorMembershipSnapshot along with the rest.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Tue, 26 Nov 2019 01:19:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Nov 2019 23:46:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 4: Code-Review+2

Rebased, carrying Tim's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Tue, 03 Dec 2019 20:11:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

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

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

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................

IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

This change improves the cluster membership snapshot we maintain in the
frontend in cases where all executors have been shut down or none have
started yet.

Prior to this change when configuring Impala with executor groups, the
planner might see a ExecutorMembershipSnapshot that has no executors in
it. This could happen if the first executor group had not started up
yet, or if all executor groups had been shutdown. If this happened, the
planner would make sub-optimal decisions, e.g. decide on a broadcast
join vs a partitioned hash join.

With this change if no executors have been registered so far, the
planner will use the expected number of executors which can be set using
the -num_expected_executors flag and is 20 by default. After executors
come online, the planner will use the size of the largest healthy
executor group, and it will hold on to the group's size even if it shuts
down or becomes unhealthy. This allows the planner to work on the
assumption that a healthy executor group of the same size will
eventually come online to execute the query.

Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
---
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
A fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 259 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14756/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/14756/2/be/src/runtime/exec-env.cc@198
PS2, Line 198:   if (!group) return;
> I think this lack of updating needs some explanation - the implications are
I went with your suggestion and I think it looks good.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Wed, 27 Nov 2019 01:07:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Nov 2019 21:44:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@18
PS1, Line 18: join vs a PHJ.
> Maybe spell out "partitioned hash join" (if that is what this is)?
Done


http://gerrit.cloudera.org:8080/#/c/14756/1//COMMIT_MSG@21
PS1, Line 21: planner will use a configurable default which should be set to the
> Maybe mention that the default is 20?
Done


http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/14756/1/be/src/service/impala-server.cc@253
PS1, Line 253: DEFINE_int32(num_expected_executors, 20, "The number of executors that the planner "
> Maybe 'the number of executors that are expected to be available for the ex
Done


http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java:

http://gerrit.cloudera.org:8080/#/c/14756/1/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@39
PS1, Line 39:   // The set of hosts that are members of the cluster given by hostname.
> Should we add some caveats here (and for ipAddresses_) that these values ar
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Nov 2019 22:45:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Wed, 27 Nov 2019 01:52:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@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: Wed, 04 Dec 2019 00:40:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

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/14756 )

Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
......................................................................

IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot

This change improves the cluster membership snapshot we maintain in the
frontend in cases where all executors have been shut down or none have
started yet.

Prior to this change when configuring Impala with executor groups, the
planner might see a ExecutorMembershipSnapshot that has no executors in
it. This could happen if the first executor group had not started up
yet, or if all executor groups had been shutdown. If this happened, the
planner would make sub-optimal decisions, e.g. decide on a broadcast
join vs a partitioned hash join.

With this change if no executors have been registered so far, the
planner will use the expected number of executors which can be set using
the -num_expected_executors flag and is 20 by default. After executors
come online, the planner will use the size of the largest healthy
executor group, and it will hold on to the group's size even if it shuts
down or becomes unhealthy. This allows the planner to work on the
assumption that a healthy executor group of the same size will
eventually come online to execute the query.

Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Reviewed-on: http://gerrit.cloudera.org:8080/14756
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/exec-env.cc
M be/src/service/impala-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
A fe/src/test/java/org/apache/impala/planner/ClusterSizeTest.java
M tests/custom_cluster/test_coordinators.py
M tests/custom_cluster/test_executor_groups.py
8 files changed, 266 insertions(+), 18 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9
Gerrit-Change-Number: 14756
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>