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 2020/01/12 00:18:55 UTC
[GitHub] [incubator-pinot] kishoreg opened a new pull request #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
kishoreg opened a new pull request #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972
…oller
@jackjlli Can you please review this? This removes the exception we are seeing. I don't see any reason to use CrushAlgorithm. It's for advanced use cases and we don't need it for Controller.
Also, why should we model it as MasterSlave instead of using a simple online/offline model? I dint bother changing the state model since it requires changes elsewhere.
Please review this
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573439225
> We expect to move to 0.9.2 in the next few weeks, if that helps.
It won't help. that code still throws that exception. I am not sure why we need such a complex rebalancing strategy - distributing tables among controllers is a fairly simple problem that can be solved with online-offline state model. See this recipe https://helix.apache.org/0.6.4-docs/recipes/lock_manager.html
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573436524
We expect to move to 0.9.2 in the next few weeks, if that helps.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573966350
Thanks @jackjlli. You can always change the rebalancer at runtime and it should work.
https://github.com/apache/helix/issues/680
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg edited a comment on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg edited a comment on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573391712
Controllers are not stateful. We don’t need affinity when a controller start/stop. Why do you say that there will more than one host in online state?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573473269
Why do we need that behavior? I can understand that behavior for a Sharded database, but there is no need for the same controller to be manage a table in case of Pinot controller.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573522215
why will debug logs be spread across multiple logs if there is no failure. And if there is a failure, it will be in different logs in both cases.
Can you start a thread on Helix mailing list. I would like to understand why online offline model won’t work.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573472545
Affinity is needed when controller's workload is distributed to multiple controllers.
E.g. suppose the leader of Partition X is host_1. After restarting host_1, we don't want the leader of partition X gets changed to other controller hosts after host_1 has been fully restarted; we want host_1 to always be the leader of Partition X. This can reduce the change of leadership change in the whole Pinot cluster. And crushED algorithm provides us this guarantee:
http://helix.apache.org/0.8.4-docs/design_crushed.html
0.9.2 will also touch that exception, but it only prints a one-line message instead of the whole stack trace. Thus, the log will be clean. And the message will not longer appear after the first controller is fully prepared.
The goal of leadControllerResource is to make sure there is at most 1 host in Master state within any partitions. OnlineOffline cannot guarantee that. E.g. if OnlineOffline state model is used, when a host gets restarted, it's possible that a state transition from offline to online gets triggered before the state transition from online to offline does, which leads to a scenario that there are two hosts in Online state, which is harmful to realtime segment completion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573473662
The information about online offline model is incorrect. Helix sends online to offline first to current leader before sending offline to online to another node.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573391712
Controller are not state full systems. We don’t need affinity when controller start/stop. Why do you say that there will more than one host in online state?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573390295
@kishoreg Thanks for inviting. While this algorithm is needed to provide even distribution and minimum partition movements instances go up/down. The AutoRebalance algorithm isn't deterministic, which doesn't guarantee the previous controller will become the one for the same partitions after restarting the host.
For MasterSlave state model, this is also needed as well, because we want to make sure there is at most 1 host being the master within the partition. OnlineOffline state model doesn't guarantee that; it's possible there are more than one host in online state within a short period of time, which is especially harmful for realtime segment completion.
I think the best approach to remove that alert/exception is to contact Helix team to unify the behaviors of all the algorithms.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573369342
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=h1) Report
> Merging [#4972](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/97a3c2161adbd62dce046cd962c42f116f9065d8?src=pr&el=desc) will **increase** coverage by `20.89%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4972 +/- ##
=============================================
+ Coverage 36.45% 57.35% +20.89%
- Complexity 0 12 +12
=============================================
Files 1173 1173
Lines 62107 62204 +97
Branches 9128 9145 +17
=============================================
+ Hits 22644 35679 +13035
+ Misses 37441 23854 -13587
- Partials 2022 2671 +649
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ot/controller/helix/core/util/HelixSetupUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3V0aWwvSGVsaXhTZXR1cFV0aWxzLmphdmE=) | `88.75% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=) | `47.22% <0%> (-8.34%)` | `0% <0%> (ø)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `39.02% <0%> (-2.44%)` | `0% <0%> (ø)` | |
| [...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==) | `94.05% <0%> (-0.55%)` | `0% <0%> (ø)` | |
| [...he/pinot/tools/admin/command/QuickstartRunner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL2FkbWluL2NvbW1hbmQvUXVpY2tzdGFydFJ1bm5lci5qYXZh) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| [.../apache/pinot/tools/streams/AirlineDataStream.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL3N0cmVhbXMvQWlybGluZURhdGFTdHJlYW0uamF2YQ==) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/tools/Quickstart.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL1F1aWNrc3RhcnQuamF2YQ==) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| [.../java/org/apache/pinot/tools/HybridQuickstart.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL0h5YnJpZFF1aWNrc3RhcnQuamF2YQ==) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| [...ools/admin/command/StreamAvroIntoKafkaCommand.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL2FkbWluL2NvbW1hbmQvU3RyZWFtQXZyb0ludG9LYWZrYUNvbW1hbmQuamF2YQ==) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| [...org/apache/pinot/tools/QuickstartTableRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL1F1aWNrc3RhcnRUYWJsZVJlcXVlc3QuamF2YQ==) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
| ... and [590 more](https://codecov.io/gh/apache/incubator-pinot/pull/4972/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=footer). Last update [97a3c21...0146938](https://codecov.io/gh/apache/incubator-pinot/pull/4972?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
jackjlli commented on issue #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972#issuecomment-573507318
That behavior is quite useful especially when some issue needs to be debugged; we don't want the logging info of a table spreads across all the controllers.
As for OnlineOffline state model, it won't restrict the online replica to be 1. This info is confirmed by Helix team and they recommend using MasterSlave/LeadStandby instead of OnlineOffline model.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg merged pull request #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
Posted by GitBox <gi...@apache.org>.
kishoreg merged pull request #4972: Changing the rebalance strategy from Crush to AutoRebalance for contr…
URL: https://github.com/apache/incubator-pinot/pull/4972
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org