[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Tested-by: Impala Public Jenkins --- 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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 18 Mar 2021 00:39:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 17 Mar 2021 18:55:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 17 Mar 2021 18:55:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 17 Mar 2021 06:03:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 17 Mar 2021 05:54:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Wed, 17 Mar 2021 05:46:56 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 17 Mar 2021 05:34:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Mon, 15 Mar 2021 21:07:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 22:09:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 21:04:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 20:46:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 18:07:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 04:10:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 04:06:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 13 Mar 2021 04:00:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-10518: Add ImpalaServer interface to retrieve executor membership.
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 13 Mar 2021 03:55:47 + Gerrit-HasComments: Yes
[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
