You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/10/11 19:20:00 UTC
[GitHub] [pinot] chenboat opened a new pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
chenboat opened a new pull request #7556:
URL: https://github.com/apache/pinot/pull/7556
## Description
Fix the issue described in #7491. Today the controller getTableInstance api gets the brokers serving a table based on the table's broker tenant config. It fetches the brokers list based only on the tenant info. Some of the brokers in the returned list may not in fact serve the table if they are just added to the tenant. This PR fixes the issue by fetching the brokers from the broker external view.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6ac7f45) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `2.81%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 69.86% -2.82%
+ Complexity 3413 3338 -75
============================================
Files 1524 1127 -397
Lines 75702 53662 -22040
Branches 11039 8107 -2932
============================================
- Hits 55017 37489 -17528
+ Misses 16995 13532 -3463
+ Partials 3690 2641 -1049
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.86% <ø> (-0.04%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ot/common/restlet/resources/TableMetadataInfo.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVNZXRhZGF0YUluZm8uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [601 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...6ac7f45](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed67337) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `1.48%`.
> The diff coverage is `56.25%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 71.19% -1.49%
- Complexity 3413 3418 +5
============================================
Files 1524 1522 -2
Lines 75702 75915 +213
Branches 11039 11096 +57
============================================
- Hits 55017 54047 -970
- Misses 16995 18186 +1191
+ Partials 3690 3682 -8
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `28.83% <31.25%> (-0.31%)` | :arrow_down: |
| unittests1 | `69.86% <ø> (-0.03%)` | :arrow_down: |
| unittests2 | `15.21% <56.25%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `64.69% <0.00%> (-0.23%)` | :arrow_down: |
| [...va/org/apache/pinot/client/ExternalViewReader.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4dGVybmFsVmlld1JlYWRlci5qYXZh) | `82.19% <100.00%> (+1.30%)` | :arrow_up: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-78.27%)` | :arrow_down: |
| ... and [127 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...ed67337](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e8a14aa) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `7.37%`.
> The diff coverage is `76.63%`.
> :exclamation: Current head e8a14aa differs from pull request most recent head 4e871fb. Consider uploading reports for the commit 4e871fb to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 65.29% -7.38%
- Complexity 3413 4000 +587
============================================
Files 1524 1529 +5
Lines 75702 78077 +2375
Branches 11039 11640 +601
============================================
- Hits 55017 50982 -4035
- Misses 16995 23477 +6482
+ Partials 3690 3618 -72
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `68.77% <77.25%> (-1.13%)` | :arrow_down: |
| unittests2 | `14.55% <9.69%> (-0.72%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <ø> (-78.27%)` | :arrow_down: |
| [.../minion/RealtimeToOfflineSegmentsTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL1JlYWx0aW1lVG9PZmZsaW5lU2VnbWVudHNUYXNrTWV0YWRhdGEuamF2YQ==) | `100.00% <ø> (+26.66%)` | :arrow_up: |
| [...e/pinot/common/utils/FileUploadDownloadClient.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRmlsZVVwbG9hZERvd25sb2FkQ2xpZW50LmphdmE=) | `19.04% <0.00%> (-47.62%)` | :arrow_down: |
| [...ache/pinot/common/utils/ServiceStartableUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VydmljZVN0YXJ0YWJsZVV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...java/org/apache/pinot/common/utils/StringUtil.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU3RyaW5nVXRpbC5qYXZh) | `100.00% <ø> (+23.52%)` | :arrow_up: |
| [...apache/pinot/common/utils/config/TagNameUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvY29uZmlnL1RhZ05hbWVVdGlscy5qYXZh) | `60.71% <0.00%> (-35.59%)` | :arrow_down: |
| [...g/apache/pinot/common/utils/helix/HelixHelper.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvSGVsaXhIZWxwZXIuamF2YQ==) | `16.01% <0.00%> (-32.26%)` | :arrow_down: |
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ore/assignment/segment/SegmentAssignmentUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudFV0aWxzLmphdmE=) | `99.39% <ø> (ø)` | |
| [...troller/helix/core/minion/ClusterInfoAccessor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9DbHVzdGVySW5mb0FjY2Vzc29yLmphdmE=) | `42.30% <0.00%> (-47.35%)` | :arrow_down: |
| ... and [609 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...4e871fb](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r738958354
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,27 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableNameWithType}/livebrokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getLiveBrokersForTable(
+ @ApiParam(value = "Table name with type", required = true)
+ @PathParam("tableNameWithType") String tableNameWithType) {
+ ObjectNode ret = JsonUtils.newObjectNode();
+ ret.put("tableName", tableNameWithType);
+ List<String> liveBrokersForTable = _pinotHelixResourceManager.getLiveBrokersForTable(tableNameWithType);
Review comment:
Let's directly `return _pinotHelixResourceManager.getLiveBrokersForTable(tableNameWithType);` instead of constructing a json object. User should expect a list per the API definition.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
Review comment:
(minor) Use javadoc comment block
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
+ // Each entry in the broker list is of the following format:
+ // Broker_hostname_port
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
Review comment:
(optional) Suggest making the input `tableName` (table name with or without type). If the input is raw table name, search for both `OFFLINE` and `REALTIME` table live brokers and intersect them.
This can be done in a separate PR.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
+ // Each entry in the broker list is of the following format:
+ // Broker_hostname_port
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalView ev = _helixDataAccessor.getProperty(_keyBuilder.externalView(Helix.BROKER_RESOURCE_INSTANCE));
+ if (ev == null) {
+ return Collections.EMPTY_LIST;
+ }
+ Map<String, String> brokerToStateMap = ev.getStateMap(tableNameWithType);
+ List<String> hosts = new ArrayList<>();
+ if (brokerToStateMap != null) {
+ for (Map.Entry<String, String> entry : brokerToStateMap.entrySet()) {
+ if ("ONLINE".equalsIgnoreCase(entry.getValue())) {
+ hosts.add(entry.getKey());
+ }
+ }
+ }
+ return hosts;
+ }
Review comment:
(minor) Add an empty line following the method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e871fb) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `1.01%`.
> The diff coverage is `15.78%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 71.65% -1.02%
- Complexity 3413 3938 +525
============================================
Files 1524 1575 +51
Lines 75702 79929 +4227
Branches 11039 11840 +801
============================================
+ Hits 55017 57277 +2260
- Misses 16995 18790 +1795
- Partials 3690 3862 +172
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `29.04% <15.78%> (-1.70%)` | :arrow_down: |
| integration2 | `27.77% <0.00%> (-1.37%)` | :arrow_down: |
| unittests1 | `68.74% <ø> (-1.15%)` | :arrow_down: |
| unittests2 | `14.54% <15.78%> (-0.73%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `65.37% <30.00%> (+0.45%)` | :arrow_up: |
| [...ache/pinot/plugin/metrics/yammer/YammerMetric.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1tZXRyaWNzL3Bpbm90LXlhbW1lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL21ldHJpY3MveWFtbWVyL1lhbW1lck1ldHJpYy5qYXZh) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-29.42%)` | :arrow_down: |
| [.../pinot/plugin/metrics/yammer/YammerMetricName.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1tZXRyaWNzL3Bpbm90LXlhbW1lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL21ldHJpY3MveWFtbWVyL1lhbW1lck1ldHJpY05hbWUuamF2YQ==) | `33.33% <0.00%> (-20.00%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `66.33% <0.00%> (-18.99%)` | :arrow_down: |
| [...pinot/common/utils/fetcher/HttpSegmentFetcher.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZmV0Y2hlci9IdHRwU2VnbWVudEZldGNoZXIuamF2YQ==) | `51.72% <0.00%> (-17.25%)` | :arrow_down: |
| [...t/plugin/metrics/yammer/YammerMetricsRegistry.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1tZXRyaWNzL3Bpbm90LXlhbW1lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL21ldHJpY3MveWFtbWVyL1lhbW1lck1ldHJpY3NSZWdpc3RyeS5qYXZh) | `60.00% <0.00%> (-12.00%)` | :arrow_down: |
| [...he/pinot/segment/local/segment/store/IndexKey.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L3N0b3JlL0luZGV4S2V5LmphdmE=) | `65.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `53.46% <0.00%> (-8.92%)` | :arrow_down: |
| ... and [251 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...4e871fb](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r738770848
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,14 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ BrokerResourceExternalViewReader externalViewReader =
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed67337) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `6.83%`.
> The diff coverage is `56.25%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 65.84% -6.84%
- Complexity 3413 3418 +5
============================================
Files 1524 1476 -48
Lines 75702 74064 -1638
Branches 11039 10896 -143
============================================
- Hits 55017 48764 -6253
- Misses 16995 21846 +4851
+ Partials 3690 3454 -236
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.86% <ø> (-0.03%)` | :arrow_down: |
| unittests2 | `15.21% <56.25%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.70% <0.00%> (-4.22%)` | :arrow_down: |
| [...va/org/apache/pinot/client/ExternalViewReader.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4dGVybmFsVmlld1JlYWRlci5qYXZh) | `80.82% <100.00%> (-0.07%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [366 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...ed67337](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f13b213) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `7.50%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 65.17% -7.51%
- Complexity 3413 4021 +608
============================================
Files 1524 1532 +8
Lines 75702 78358 +2656
Branches 11039 11706 +667
============================================
- Hits 55017 51069 -3948
- Misses 16995 23652 +6657
+ Partials 3690 3637 -53
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `68.61% <ø> (-1.28%)` | :arrow_down: |
| unittests2 | `14.54% <60.00%> (-0.73%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `59.98% <64.28%> (-4.94%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...not/common/exception/HttpErrorStatusException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL0h0dHBFcnJvclN0YXR1c0V4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [546 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...f13b213](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ed67337) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `57.46%`.
> The diff coverage is `56.25%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
=============================================
- Coverage 72.67% 15.21% -57.47%
+ Complexity 3413 80 -3333
=============================================
Files 1524 1476 -48
Lines 75702 74064 -1638
Branches 11039 10896 -143
=============================================
- Hits 55017 11269 -43748
- Misses 16995 61979 +44984
+ Partials 3690 816 -2874
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.21% <56.25%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.70% <0.00%> (-4.22%)` | :arrow_down: |
| [...va/org/apache/pinot/client/ExternalViewReader.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4dGVybmFsVmlld1JlYWRlci5qYXZh) | `80.82% <100.00%> (-0.07%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1209 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...ed67337](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat merged pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat merged pull request #7556:
URL: https://github.com/apache/pinot/pull/7556
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727392927
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
Returning empty map.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
Revised to return empty map.
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
Review comment:
done.
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
+ }
+
+ // Return a map from tablename (with type) to live brokers (of raw instance format broker_host_port).
+ public Map<String, List<String>> getTableWithTypeToRawBrokerInstanceIdsMap() {
+ return getTableToBrokersMapFromExternalView(true, false);
+ }
+
+ private Map<String, List<String>> getTableToBrokersMapFromExternalView(boolean tableWithType, boolean useUrlFormat) {
Map<String, Set<String>> brokerUrlsMap = new HashMap<>();
try {
byte[] brokerResourceNodeData = _zkClient.readData("/EXTERNALVIEW/brokerResource", true);
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727502324
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -68,7 +69,8 @@ public String getTableInstances(
ObjectNode e = JsonUtils.newObjectNode();
e.put("tableType", "offline");
ArrayNode a = JsonUtils.newArrayNode();
- for (String ins : _pinotHelixResourceManager.getBrokerInstancesForTable(tableName, TableType.OFFLINE)) {
+ for (String ins : _pinotHelixResourceManager
+ .getLiveBrokersForTable(TableNameBuilder.OFFLINE.tableNameWithType(tableName))) {
Review comment:
This will make the broker and server behavior inconsistent (currently both of them are tag based), and change the behavior of this API
##########
File path: pinot-controller/pom.xml
##########
@@ -197,6 +197,10 @@
<groupId>org.quartz-scheduler</groupId>
<artifactId>quartz</artifactId>
</dependency>
+ <dependency>
Review comment:
Pinot-controller should not include client module. We should implement it separately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r739537928
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,27 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableNameWithType}/livebrokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getLiveBrokersForTable(
+ @ApiParam(value = "Table name with type", required = true)
+ @PathParam("tableNameWithType") String tableNameWithType) {
+ ObjectNode ret = JsonUtils.newObjectNode();
+ ret.put("tableName", tableNameWithType);
+ List<String> liveBrokersForTable = _pinotHelixResourceManager.getLiveBrokersForTable(tableNameWithType);
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6ac7f45) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `6.81%`.
> The diff coverage is `53.33%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 65.86% -6.82%
- Complexity 3413 3418 +5
============================================
Files 1524 1476 -48
Lines 75702 74043 -1659
Branches 11039 10888 -151
============================================
- Hits 55017 48768 -6249
- Misses 16995 21821 +4826
+ Partials 3690 3454 -236
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.86% <ø> (-0.04%)` | :arrow_down: |
| unittests2 | `15.23% <53.33%> (-0.04%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.70% <0.00%> (-4.22%)` | :arrow_down: |
| [...va/org/apache/pinot/client/ExternalViewReader.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4dGVybmFsVmlld1JlYWRlci5qYXZh) | `80.82% <100.00%> (-0.07%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [365 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...6ac7f45](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r728652937
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e871fb) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `2.21%`.
> The diff coverage is `15.78%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 70.46% -2.22%
- Complexity 3413 3938 +525
============================================
Files 1524 1575 +51
Lines 75702 79929 +4227
Branches 11039 11840 +801
============================================
+ Hits 55017 56320 +1303
- Misses 16995 19761 +2766
- Partials 3690 3848 +158
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.77% <0.00%> (-1.37%)` | :arrow_down: |
| unittests1 | `68.74% <ø> (-1.15%)` | :arrow_down: |
| unittests2 | `14.54% <15.78%> (-0.73%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `65.29% <30.00%> (+0.37%)` | :arrow_up: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-78.27%)` | :arrow_down: |
| [...ache/pinot/common/lineage/SegmentLineageUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50TGluZWFnZVV0aWxzLmphdmE=) | `22.22% <0.00%> (-77.78%)` | :arrow_down: |
| ... and [315 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...4e871fb](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mqliang commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r726803300
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
+ }
+
+ // Return a map from tablename (with type) to live brokers (of raw instance format broker_host_port).
+ public Map<String, List<String>> getTableWithTypeToRawBrokerInstanceIdsMap() {
+ return getTableToBrokersMapFromExternalView(true, false);
+ }
+
+ private Map<String, List<String>> getTableToBrokersMapFromExternalView(boolean tableWithType, boolean useUrlFormat) {
Map<String, Set<String>> brokerUrlsMap = new HashMap<>();
try {
byte[] brokerResourceNodeData = _zkClient.readData("/EXTERNALVIEW/brokerResource", true);
Review comment:
You can reuse the BROKER_EXTERNAL_VIEW_PATH constant to replace String "/EXTERNALVIEW/brokerResource" here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727392927
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
Revised to return empty map.
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
Review comment:
done.
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
+ }
+
+ // Return a map from tablename (with type) to live brokers (of raw instance format broker_host_port).
+ public Map<String, List<String>> getTableWithTypeToRawBrokerInstanceIdsMap() {
+ return getTableToBrokersMapFromExternalView(true, false);
+ }
+
+ private Map<String, List<String>> getTableToBrokersMapFromExternalView(boolean tableWithType, boolean useUrlFormat) {
Map<String, Set<String>> brokerUrlsMap = new HashMap<>();
try {
byte[] brokerResourceNodeData = _zkClient.readData("/EXTERNALVIEW/brokerResource", true);
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727502324
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -68,7 +69,8 @@ public String getTableInstances(
ObjectNode e = JsonUtils.newObjectNode();
e.put("tableType", "offline");
ArrayNode a = JsonUtils.newArrayNode();
- for (String ins : _pinotHelixResourceManager.getBrokerInstancesForTable(tableName, TableType.OFFLINE)) {
+ for (String ins : _pinotHelixResourceManager
+ .getLiveBrokersForTable(TableNameBuilder.OFFLINE.tableNameWithType(tableName))) {
Review comment:
This will make the broker and server behavior inconsistent (currently both of them are tag based), and change the behavior of this API
##########
File path: pinot-controller/pom.xml
##########
@@ -197,6 +197,10 @@
<groupId>org.quartz-scheduler</groupId>
<artifactId>quartz</artifactId>
</dependency>
+ <dependency>
Review comment:
Pinot-controller should not include client module. We should implement it separately
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
Review comment:
```suggestion
public String getTableLiveBrokers(
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/BrokerResourceExternalViewReader.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.zip.GZIPInputStream;
+import org.I0Itec.zkclient.ZkClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Reads brokers external view from Zookeeper
+ */
+public class BrokerResourceExternalViewReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BrokerResourceExternalViewReader.class);
+ private static final ObjectReader OBJECT_READER = new ObjectMapper().reader();
+ public static final String BROKER_EXTERNAL_VIEW_PATH = "/EXTERNALVIEW/brokerResource";
+ public static final String REALTIME_SUFFIX = "_REALTIME";
+ public static final String OFFLINE_SUFFIX = "_OFFLINE";
+
+ private ZkClient _zkClient;
+
+ public BrokerResourceExternalViewReader(ZkClient zkClient) {
Review comment:
Move the logic in this class into the `PinotHelixResourceManager`. We don't need to use the `ZkClient` api to read the external view
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
+ @ApiParam(value = "Table name without type", required = true) @PathParam("tableName") String tableName) {
+ ObjectNode ret = JsonUtils.newObjectNode();
Review comment:
Let's not reuse the response format for this new API. Would suggest just returning an array of live brokers.
- If the input is a table name with type, returns the live brokers for the table, or 404 if table is not found
- If the input is a raw table name, if both offline and realtime table exist, return the brokers live for both tables (we should only route query to the broker with both tables online); if only one table exist, return the live brokers for the single table; if no table is found, return 404
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,14 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ BrokerResourceExternalViewReader externalViewReader =
Review comment:
This can be simplified as you have access to helix property in this class. You can get the `brokerResource` external view by calling `_helixDataAccessor.getProperty(_keyBuilder.externalView(CommonConstants.Helix.BROKER_RESOURCE_INSTANCE))`
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
Review comment:
Suggest changing it to `/tables/{tableName}/livebrokers` to differentiate it from the other one
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,14 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ BrokerResourceExternalViewReader externalViewReader =
Review comment:
The table existence check can be pushed here so that we only read the external view once
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/BrokerResourceExternalViewReader.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.zip.GZIPInputStream;
+import org.I0Itec.zkclient.ZkClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Reads brokers external view from Zookeeper
+ */
+public class BrokerResourceExternalViewReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BrokerResourceExternalViewReader.class);
+ private static final ObjectReader OBJECT_READER = new ObjectMapper().reader();
+ public static final String BROKER_EXTERNAL_VIEW_PATH = "/EXTERNALVIEW/brokerResource";
+ public static final String REALTIME_SUFFIX = "_REALTIME";
+ public static final String OFFLINE_SUFFIX = "_OFFLINE";
+
+ private ZkClient _zkClient;
+
+ public BrokerResourceExternalViewReader(ZkClient zkClient) {
+ _zkClient = zkClient;
+ }
+
+ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
+ return new ByteArrayInputStream(brokerResourceNodeData);
+ }
+
+ // Return a map from tablename (without type) to live brokers (of format host:port).
+ public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+ }
+
+ // Return a map from tablename (with type) to live brokers (of raw instance format broker_host_port).
+ public Map<String, List<String>> getTableWithTypeToRawBrokerInstanceIdsMap() {
+ return getTableToBrokersMapFromExternalView(true, false);
+ }
+
+ private Map<String, List<String>> getTableToBrokersMapFromExternalView(boolean tableWithType, boolean useUrlFormat) {
+ Map<String, Set<String>> brokerUrlsMap = new HashMap<>();
+ try {
+ byte[] brokerResourceNodeData = _zkClient.readData(BROKER_EXTERNAL_VIEW_PATH, true);
+ brokerResourceNodeData = unpackZnodeIfNecessary(brokerResourceNodeData);
+ JsonNode jsonObject = OBJECT_READER.readTree(getInputStream(brokerResourceNodeData));
+ JsonNode brokerResourceNode = jsonObject.get("mapFields");
+
+ Iterator<Map.Entry<String, JsonNode>> resourceEntries = brokerResourceNode.fields();
+ while (resourceEntries.hasNext()) {
+ Map.Entry<String, JsonNode> resourceEntry = resourceEntries.next();
+ String resourceName = resourceEntry.getKey();
+ String tableName =
+ tableWithType ? resourceName : resourceName.replace(OFFLINE_SUFFIX, "").replace(REALTIME_SUFFIX, "");
+ Set<String> brokerUrls = brokerUrlsMap.computeIfAbsent(tableName, k -> new HashSet<>());
+ JsonNode resource = resourceEntry.getValue();
+ Iterator<Map.Entry<String, JsonNode>> brokerEntries = resource.fields();
+ while (brokerEntries.hasNext()) {
+ Map.Entry<String, JsonNode> brokerEntry = brokerEntries.next();
+ String brokerName = brokerEntry.getKey();
+ if (brokerName.startsWith("Broker_") && "ONLINE".equals(brokerEntry.getValue().asText())) {
Review comment:
The prefix check should be removed as it won't work if the broker name is customized
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] yupeng9 commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
yupeng9 commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727268599
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
Review comment:
nil: newline
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
shall we return empty map other than null?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r728309745
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
Review comment:
```suggestion
public String getTableLiveBrokers(
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/BrokerResourceExternalViewReader.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.zip.GZIPInputStream;
+import org.I0Itec.zkclient.ZkClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Reads brokers external view from Zookeeper
+ */
+public class BrokerResourceExternalViewReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BrokerResourceExternalViewReader.class);
+ private static final ObjectReader OBJECT_READER = new ObjectMapper().reader();
+ public static final String BROKER_EXTERNAL_VIEW_PATH = "/EXTERNALVIEW/brokerResource";
+ public static final String REALTIME_SUFFIX = "_REALTIME";
+ public static final String OFFLINE_SUFFIX = "_OFFLINE";
+
+ private ZkClient _zkClient;
+
+ public BrokerResourceExternalViewReader(ZkClient zkClient) {
Review comment:
Move the logic in this class into the `PinotHelixResourceManager`. We don't need to use the `ZkClient` api to read the external view
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
+ @ApiParam(value = "Table name without type", required = true) @PathParam("tableName") String tableName) {
+ ObjectNode ret = JsonUtils.newObjectNode();
Review comment:
Let's not reuse the response format for this new API. Would suggest just returning an array of live brokers.
- If the input is a table name with type, returns the live brokers for the table, or 404 if table is not found
- If the input is a raw table name, if both offline and realtime table exist, return the brokers live for both tables (we should only route query to the broker with both tables online); if only one table exist, return the live brokers for the single table; if no table is found, return 404
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,14 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ BrokerResourceExternalViewReader externalViewReader =
Review comment:
This can be simplified as you have access to helix property in this class. You can get the `brokerResource` external view by calling `_helixDataAccessor.getProperty(_keyBuilder.externalView(CommonConstants.Helix.BROKER_RESOURCE_INSTANCE))`
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
Review comment:
Suggest changing it to `/tables/{tableName}/livebrokers` to differentiate it from the other one
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,14 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ BrokerResourceExternalViewReader externalViewReader =
Review comment:
The table existence check can be pushed here so that we only read the external view once
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/BrokerResourceExternalViewReader.java
##########
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.helix.core.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.zip.GZIPInputStream;
+import org.I0Itec.zkclient.ZkClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Reads brokers external view from Zookeeper
+ */
+public class BrokerResourceExternalViewReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BrokerResourceExternalViewReader.class);
+ private static final ObjectReader OBJECT_READER = new ObjectMapper().reader();
+ public static final String BROKER_EXTERNAL_VIEW_PATH = "/EXTERNALVIEW/brokerResource";
+ public static final String REALTIME_SUFFIX = "_REALTIME";
+ public static final String OFFLINE_SUFFIX = "_OFFLINE";
+
+ private ZkClient _zkClient;
+
+ public BrokerResourceExternalViewReader(ZkClient zkClient) {
+ _zkClient = zkClient;
+ }
+
+ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
+ return new ByteArrayInputStream(brokerResourceNodeData);
+ }
+
+ // Return a map from tablename (without type) to live brokers (of format host:port).
+ public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+ }
+
+ // Return a map from tablename (with type) to live brokers (of raw instance format broker_host_port).
+ public Map<String, List<String>> getTableWithTypeToRawBrokerInstanceIdsMap() {
+ return getTableToBrokersMapFromExternalView(true, false);
+ }
+
+ private Map<String, List<String>> getTableToBrokersMapFromExternalView(boolean tableWithType, boolean useUrlFormat) {
+ Map<String, Set<String>> brokerUrlsMap = new HashMap<>();
+ try {
+ byte[] brokerResourceNodeData = _zkClient.readData(BROKER_EXTERNAL_VIEW_PATH, true);
+ brokerResourceNodeData = unpackZnodeIfNecessary(brokerResourceNodeData);
+ JsonNode jsonObject = OBJECT_READER.readTree(getInputStream(brokerResourceNodeData));
+ JsonNode brokerResourceNode = jsonObject.get("mapFields");
+
+ Iterator<Map.Entry<String, JsonNode>> resourceEntries = brokerResourceNode.fields();
+ while (resourceEntries.hasNext()) {
+ Map.Entry<String, JsonNode> resourceEntry = resourceEntries.next();
+ String resourceName = resourceEntry.getKey();
+ String tableName =
+ tableWithType ? resourceName : resourceName.replace(OFFLINE_SUFFIX, "").replace(REALTIME_SUFFIX, "");
+ Set<String> brokerUrls = brokerUrlsMap.computeIfAbsent(tableName, k -> new HashSet<>());
+ JsonNode resource = resourceEntry.getValue();
+ Iterator<Map.Entry<String, JsonNode>> brokerEntries = resource.fields();
+ while (brokerEntries.hasNext()) {
+ Map.Entry<String, JsonNode> brokerEntry = brokerEntries.next();
+ String brokerName = brokerEntry.getKey();
+ if (brokerName.startsWith("Broker_") && "ONLINE".equals(brokerEntry.getValue().asText())) {
Review comment:
The prefix check should be removed as it won't work if the broker name is customized
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r736998733
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
+ @ApiParam(value = "Table name without type", required = true) @PathParam("tableName") String tableName) {
+ ObjectNode ret = JsonUtils.newObjectNode();
Review comment:
Refactor to return the list of live brokers given a tablename with type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727392927
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
Returning empty map.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] yupeng9 commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
yupeng9 commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r727268599
##########
File path: pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ExternalViewReader.java
##########
@@ -87,7 +87,18 @@ protected ByteArrayInputStream getInputStream(byte[] brokerResourceNodeData) {
return new ByteArrayInputStream(brokerResourceNodeData);
}
+ // Return a map from tablename (without type) to live brokers (of format host:port).
public Map<String, List<String>> getTableToBrokersMap() {
+ return getTableToBrokersMapFromExternalView(false, true);
+
Review comment:
nil: newline
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2864,13 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving a table. Each entry is of the following format:
+ // Broker_hostname_port (e.g., broker_12.34.56.78_1234)
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalViewReader externalViewReader = new ExternalViewReader(new ZkClient(_helixZkURL));
+ Map<String, List<String>> tableToBrokersMap = externalViewReader.getTableWithTypeToRawBrokerInstanceIdsMap();
+ return tableToBrokersMap == null ? null : tableToBrokersMap.get(tableNameWithType);
Review comment:
shall we return empty map other than null?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e8a14aa) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `58.11%`.
> The diff coverage is `9.69%`.
> :exclamation: Current head e8a14aa differs from pull request most recent head 4e871fb. Consider uploading reports for the commit 4e871fb to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
=============================================
- Coverage 72.67% 14.55% -58.12%
+ Complexity 3413 80 -3333
=============================================
Files 1524 1529 +5
Lines 75702 78077 +2375
Branches 11039 11640 +601
=============================================
- Hits 55017 11367 -43650
- Misses 16995 65895 +48900
+ Partials 3690 815 -2875
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.55% <9.69%> (-0.72%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...inot/common/function/scalar/DateTimeFunctions.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0RhdGVUaW1lRnVuY3Rpb25zLmphdmE=) | `0.00% <0.00%> (-98.67%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/PinotMetricUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9QaW5vdE1ldHJpY1V0aWxzLmphdmE=) | `0.00% <0.00%> (-88.16%)` | :arrow_down: |
| [...g/apache/pinot/common/minion/BaseTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL0Jhc2VUYXNrTWV0YWRhdGEuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <ø> (-78.27%)` | :arrow_down: |
| [...e/pinot/common/minion/MinionTaskMetadataUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01pbmlvblRhc2tNZXRhZGF0YVV0aWxzLmphdmE=) | `0.00% <0.00%> (-71.43%)` | :arrow_down: |
| [.../minion/RealtimeToOfflineSegmentsTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL1JlYWx0aW1lVG9PZmZsaW5lU2VnbWVudHNUYXNrTWV0YWRhdGEuamF2YQ==) | `0.00% <ø> (-73.34%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-85.32%)` | :arrow_down: |
| [...java/org/apache/pinot/common/utils/DataSchema.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRGF0YVNjaGVtYS5qYXZh) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| ... and [1424 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...4e871fb](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r739538442
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
+ // Each entry in the broker list is of the following format:
+ // Broker_hostname_port
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
Review comment:
ack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f13b213) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `58.12%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
=============================================
- Coverage 72.67% 14.54% -58.13%
+ Complexity 3413 80 -3333
=============================================
Files 1524 1532 +8
Lines 75702 78358 +2656
Branches 11039 11706 +667
=============================================
- Hits 55017 11401 -43616
- Misses 16995 66132 +49137
+ Partials 3690 825 -2865
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.54% <60.00%> (-0.73%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `59.98% <64.28%> (-4.94%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1279 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...f13b213](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7556](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f13b213) into [master](https://codecov.io/gh/apache/pinot/commit/9ef1f490fbbbab645a467b4464cf6c7d05a88ae2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ef1f49) will **decrease** coverage by `2.27%`.
> The diff coverage is `60.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7556 +/- ##
============================================
- Coverage 72.67% 70.39% -2.28%
- Complexity 3413 4021 +608
============================================
Files 1524 1578 +54
Lines 75702 80219 +4517
Branches 11039 11908 +869
============================================
+ Hits 55017 56473 +1456
- Misses 16995 19878 +2883
- Partials 3690 3868 +178
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.73% <0.00%> (-1.41%)` | :arrow_down: |
| unittests1 | `68.61% <ø> (-1.28%)` | :arrow_down: |
| unittests2 | `14.54% <60.00%> (-0.73%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../controller/api/resources/PinotTableInstances.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVJbnN0YW5jZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `63.84% <64.28%> (-1.08%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...nverttorawindex/ConvertToRawIndexTaskExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvY29udmVydHRvcmF3aW5kZXgvQ29udmVydFRvUmF3SW5kZXhUYXNrRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...plugin/segmentuploader/SegmentUploaderDefault.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1zZWdtZW50LXVwbG9hZGVyL3Bpbm90LXNlZ21lbnQtdXBsb2FkZXItZGVmYXVsdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL3NlZ21lbnR1cGxvYWRlci9TZWdtZW50VXBsb2FkZXJEZWZhdWx0LmphdmE=) | `0.00% <0.00%> (-87.10%)` | :arrow_down: |
| [...ore/startree/executor/StarTreeGroupByExecutor.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9leGVjdXRvci9TdGFyVHJlZUdyb3VwQnlFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| [.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [...ot/common/messages/RoutingTableRebuildMessage.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvUm91dGluZ1RhYmxlUmVidWlsZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [...e/pinot/common/minion/MergeRollupTaskMetadata.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWluaW9uL01lcmdlUm9sbHVwVGFza01ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-78.27%)` | :arrow_down: |
| [...ache/pinot/common/lineage/SegmentLineageUtils.java](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbGluZWFnZS9TZWdtZW50TGluZWFnZVV0aWxzLmphdmE=) | `22.22% <0.00%> (-77.78%)` | :arrow_down: |
| ... and [336 more](https://codecov.io/gh/apache/pinot/pull/7556/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [9ef1f49...f13b213](https://codecov.io/gh/apache/pinot/pull/7556?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a change in pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
chenboat commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r728650171
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,45 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API, so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableName}/brokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getTableBrokers(
+ @ApiParam(value = "Table name without type", required = true) @PathParam("tableName") String tableName) {
+ ObjectNode ret = JsonUtils.newObjectNode();
Review comment:
I think we should make this API to return as close to what the zookeeper stores as possible. Today the zk stores one typed table entries. It is better to returns the typed table entries as well. In this way this API can serve multiple purposes than just querying routing. For example, one may even use it to detect if there is any problem with hybrid table's broker assignment. We can leave it to the client to do what they want to do with the returned results -- they can do intersection if they want to.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7556: Let the controller getTableInstance() call to return the list of live brokers of a table.
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#issuecomment-940454729
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org