Introduce auto bundle split and unloading of split bundle in ModularLoadManager #857
Conversation
rdhabalia
added
type/enhancement
|
Nice! I will take a look and test it soon. |
| @@ -838,7 +838,8 @@ public void unloadNamespaceBundle(@PathParam("property") String property, @PathP | |||
| @ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have admin permission") }) | |||
| public void splitNamespaceBundle(@PathParam("property") String property, @PathParam("cluster") String cluster, | |||
| @PathParam("namespace") String namespace, @PathParam("bundle") String bundleRange, | |||
| @QueryParam("authoritative") @DefaultValue("false") boolean authoritative) { | |||
| @QueryParam("authoritative") @DefaultValue("false") boolean authoritative, | |||
| @QueryParam("unload") @DefaultValue("false") boolean unload) { | |||
There was a problem hiding this comment.
Since the brokers already have the loadBalancerAutoUnloadSplitBundlesEnabled parameter, wouldn't this unload option be redundant?
Just trigger the split and broker will decide whether to unload or not.
There was a problem hiding this comment.
That's correct but this admin-api is triggered by load-balancer and cli-tool. Sometimes we might want to just split the bundle using cli-tool without unloading it. Therefore, this query-param can give flexibility to perform accordingly.??
| return; | ||
| } | ||
| // Value may be changed dynamically. | ||
| if (conf.getLoadBalancerAutoBundleSplitEnabled()) { |
There was a problem hiding this comment.
if (!conf.getLoadBalancerAutoBundleSplitEnabled()) {
return;
}There was a problem hiding this comment.
yes, will fix it.
| } | ||
| // Value may be changed dynamically. | ||
| if (conf.getLoadBalancerAutoBundleSplitEnabled()) { | ||
| log.info("Check bundle-split");// TODO: remove this check |
| @@ -41,6 +41,8 @@ | |||
| import java.util.regex.Pattern; | |||
| import java.util.stream.Collectors; | |||
|
|
|||
| import javax.validation.constraints.Future; | |||
There was a problem hiding this comment.
Probably this is not the correct import class
There was a problem hiding this comment.
yes, this import is not being used, will remove it.
|
@rdhabalia I'm getting one exception reported even though the split itself suceeds: |
after splitting bundle we delete old bundle-data.It seems this exception happens while deleting the old-bundle-znode. I have added this logic in the end after testing this PR change. let me a take look of it. |
Uhm, I'm still seeing that being present eg: All the bundles data is still in ZK some time after the splits (In this case I've started with 1 bundle and let the auto-split take over). Same thing about the bundles list : $ bin/pulsar-admin brokers namespaces --url localhost:8080 standalone
{
"sample/standalone/test-3/0x1fffffff_0x2fffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : true
},
"sample/standalone/test-3/0xcfffffff_0xdfffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : true
},
"sample/standalone/test-3/0x1fffffff_0x3fffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : false
},
"pulsar/standalone/localhost:8080/0x00000000_0xffffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : true
},
"sample/standalone/test-3/0x7fffffff_0xffffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : false
},
"sample/standalone/test-3/0x00000000_0x0fffffff" : {
"broker_assignment" : "shared",
"is_controlled" : false,
"is_active" : true
},
...This I think is not related to this PR, though something we might want to think about. At the same time, the locks for the pre-split bundles are still there, but I think that is ok. |
sorry, I couldn't understand it. This exception was related to this PR only. Now, if bundle split happens even before leader-resource-quota created s bundle-data then bundle-data-znode will not be present. So, leader should not try to delete znode which doesn't exist.
After this fix I have tested again with 3 brokers and started with 1 bundle in namespace, I don't see exception in broker-log and also don't see bundle-data into zookeeper. Are you still able to see bundle-data with clean bundle-split by load-balancer?
I didn't understand it fully. But what happens here:
So, all old split-bundles will be disappeared once that broker will restart. We can't remove it from broker immediately because in step-2 if broker-2 may not receive watch and in that case we want broker-2 to redirect request to broker-1 by thinking that old-split-bundle still owned by broker-1. |
Correct, though we could clean them up after a certain time. We should at least be guaranteed that the other broker will have received the notification within a |
|
@rdhabalia Anyway, I was meaning that this last part (cleaning up the bundles ownership) is unreleated to this PR. |
Yes, but many times, we have seen (eg: replication-cluster change) that broker doesn't get watch. I will make this change and test this scenario in separate PR then. |
|
@rdhabalia there's one test failure, probably intermittent, but maybe related to PR. https://builds.apache.org/job/pulsar-pull-request/org.apache.pulsar$pulsar-broker/630/testReport/junit/org.apache.pulsar.client.api/BrokerServiceLookupTest/testSplitUnloadLookupTest/ |
|
yes, checking it.. |
…ix testModularLoadManagerSplitBundle
Motivation
As described in #385
ModularLoadManagerImpl lacked bundle split capabilities. Additionally, automatic bundle splits reassigned the bundles to the same broker when the bundle could more usefully be reassigned after being split.
Modifications
This PR has same logic as #385 to find out bundles which are eligible for splitting. But this PR have additional changes which we haven't addressed in #385
Result
ModularLoadManager can auto split and unload bundles.