You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2021/03/13 03:54:59 UTC
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17181
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the
current executor membership snapshot from impalad. This involves
sending a thrift request to impalad and receiving a thrift
response. Refactored some code in exec-env into a separate
function in the impala namespace which makes it easier
to populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
---
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
7 files changed, 118 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17181/1
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8381/ : 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/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 06:03:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py@746
PS3, Line 746: e
flake8: E501 line too long (101 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 20:46:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17181/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/1/tests/hs2/test_hs2.py@746
PS1, Line 746:
> flake8: E501 line too long (101 > 90 characters)
Done
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 04:00:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 2:
The build failures seem to be due to run_clang_tidy.sh. It is quite verbose so maybe hold off on the code review for now. I am not familiar with the clang_tidy output so it might take some time.
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 18:07:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Kurt Deschler, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17181
to look at the new patch set (#5).
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the current
executor membership snapshot from impalad for use by an external
frontend. This involves sending a thrift request to impalad and
receiving a thrift response. Refactored some code in exec-env into
a separate function in the impala namespace which makes it easier to
populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot.
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
---
M be/src/rpc/hs2-http-test.cc
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
8 files changed, 124 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17181/5
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Kurt Deschler, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17181
to look at the new patch set (#4).
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the
current executor membership snapshot from impalad. This involves
sending a thrift request to impalad and receiving a thrift
response. Refactored some code in exec-env into a separate
function in the impala namespace which makes it easier
to populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot.
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
---
M be/src/rpc/hs2-http-test.cc
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
8 files changed, 124 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17181/4
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 3:
> Patch Set 2:
>
> The build failures seem to be due to run_clang_tidy.sh. It is quite verbose so maybe hold off on the code review for now. I am not familiar with the clang_tidy output so it might take some time.
Fixed the clang_tidy reported error.
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 22:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17181/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/1/tests/hs2/test_hs2.py@746
PS1, Line 746: e
flake8: E501 line too long (101 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 03:55:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8356/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 04:10:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17181/4/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/4/tests/hs2/test_hs2.py@747
PS4, Line 747: g
flake8: E126 continuation line over-indented for hanging indent
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 05:34:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Kurt Deschler, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17181
to look at the new patch set (#3).
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the
current executor membership snapshot from impalad. This involves
sending a thrift request to impalad and receiving a thrift
response. Refactored some code in exec-env into a separate
function in the impala namespace which makes it easier
to populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot.
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
---
M be/src/rpc/hs2-http-test.cc
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
8 files changed, 122 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17181/3
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Mar 2021 00:39:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8358/ : 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/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 21:04:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 5:
(7 comments)
Thanks for the review.
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.h@260
PS3, Line 260: };
> Its standard in Impala to put all forward declarations at the top of the fi
Done
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.cc@57
PS3, Line 57: tring lo
> You can just put this function in the "namespace impala{" block below and r
Done
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/service/impala-hs2-server.cc@1230
PS3, Line 1230:
: // Populate an instance of TUpdate
> nit: I think this can fit on a single line
That's good to know.. I ran that tool and it certainly improved the formatting in a few places.
http://gerrit.cloudera.org:8080/#/c/17181/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/17181/3/common/thrift/ImpalaService.thrift@870
PS3, Line 870: // Returns the executor membership information. Only supported for the "external fe"
> Note that this is only supported for the external fe service
Done
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py@746
PS3, Line 746:
> flake8: E501 line too long (101 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py@750
PS3, Line 750:
> I'm confused as to how we would get the correct value here if the response
Hmm..this line was not present in patch set 1 and 2 but appeared in PS 3 because of some local changes I was trying. It's not supposed to be there. Thanks for pointing it out.
http://gerrit.cloudera.org:8080/#/c/17181/4/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/4/tests/hs2/test_hs2.py@747
PS4, Line 747: e
> flake8: E126 continuation line over-indented for hanging indent
Done
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 05:46:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall, Kurt Deschler, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17181
to look at the new patch set (#2).
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the
current executor membership snapshot from impalad. This involves
sending a thrift request to impalad and receiving a thrift
response. Refactored some code in exec-env into a separate
function in the impala namespace which makes it easier
to populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
---
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
7 files changed, 119 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17181/2
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
This patch adds an interface to ImpalaServer to retrieve the current
executor membership snapshot from impalad for use by an external
frontend. This involves sending a thrift request to impalad and
receiving a thrift response. Refactored some code in exec-env into
a separate function in the impala namespace which makes it easier to
populate the needed information for an external frontend.
Testing:
- Ran selected tests for sanity check (no impact is expected
since this is adding a new interface):
- Frontend tests (PlannerTest, CardinalityTest)
- Backend tests under custom_cluster/test_executor_groups.py
- Manually tested with external frontend to ensure it gets
the executor membership snapshot.
Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Reviewed-on: http://gerrit.cloudera.org:8080/17181
Reviewed-by: Thomas Tauber-Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/hs2-http-test.cc
M be/src/runtime/exec-env.cc
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/cluster-membership-mgr.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.h
M common/thrift/ImpalaService.thrift
M tests/hs2/test_hs2.py
8 files changed, 124 insertions(+), 39 deletions(-)
Approvals:
Thomas Tauber-Marshall: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 18:55:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/8354/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Mar 2021 04:06:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.h@260
PS3, Line 260: class TUpdateExecutorMembershipRequest;
Its standard in Impala to put all forward declarations at the top of the file - in this case, immediately after the "namespace impala {" line
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/scheduling/cluster-membership-mgr.cc@57
PS3, Line 57: impala::
You can just put this function in the "namespace impala{" block below and remove this
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/17181/3/be/src/service/impala-hs2-server.cc@1230
PS3, Line 1230: PopulateExecutorMembershipRequest(membership_snapshot,
: return_val.executor_membership);
nit: I think this can fit on a single line
More generally, you've got a few minor formatting errors, could you run this through clang-format-diff as described here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=65868536
http://gerrit.cloudera.org:8080/#/c/17181/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:
http://gerrit.cloudera.org:8080/#/c/17181/3/common/thrift/ImpalaService.thrift@870
PS3, Line 870: // Returns the executor membership information
Note that this is only supported for the external fe service
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/17181/3/tests/hs2/test_hs2.py@750
PS3, Line 750: assert get_executor_membership_resp.executor_membership.num_executors == 3
I'm confused as to how we would get the correct value here if the response was an error, as checked in the previous line
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Mar 2021 21:07:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6978/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 18:55:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17181 )
Change subject: IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8380/ : 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/17181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie89b71f4555c368869ee7b9d6341756c60af12b5
Gerrit-Change-Number: 17181
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Mar 2021 05:54:20 +0000
Gerrit-HasComments: No