[improve][broker] PIP-192: Support delete namespace bundle admin API#19851
Conversation
ade32bd to
bb0998f
Compare
bb0998f to
1638aae
Compare
| public CompletableFuture<Boolean> checkOwnershipPresentAsync(NamespaceBundle bundle) { | ||
| if (ExtensibleLoadManagerImpl.isLoadManagerExtensionEnabled(config)) { | ||
| ExtensibleLoadManagerImpl extensibleLoadManager = ExtensibleLoadManagerImpl.get(loadManager.get()); | ||
| return extensibleLoadManager.getOwnershipAsync(Optional.empty(), bundle) |
There was a problem hiding this comment.
can we pass Optional.empty() for system topics like bsc?
There was a problem hiding this comment.
Can you explain in detail?
There was a problem hiding this comment.
If we don't pass the Optional<ServiceUnitId> topic, I think the ownership cannot be found for the load manager system topics (like bsc, broker-load, top-bundles-load topics). If this checkOwnershipPresentAsync is not used to check the load manager system topics, we are good here.
// example how we use Optional<ServiceUnitId> topic
private CompletableFuture<Optional<String>> getOwnershipAsync(Optional<ServiceUnitId> topic,
ServiceUnitId bundleUnit) {
final String bundle = bundleUnit.toString();
CompletableFuture<Optional<String>> owner;
if (topic.isPresent() && isInternalTopic(topic.get().toString())) {
owner = serviceUnitStateChannel.getChannelOwnerAsync();
} else {
owner = serviceUnitStateChannel.getOwnerAsync(bundle);
}
return owner;
There was a problem hiding this comment.
I don't think when using checkOwnershipPresentAsync method, we can get the topic info.
There was a problem hiding this comment.
I think we can remove Optional<ServiceUnitId> topic and pass just bundleUnit only to getOwnershipAsync, like the one below. Let the leader own all system namespace topics, and we don't auto-unload system topics.
private CompletableFuture<Optional<String>> getOwnershipAsync(ServiceUnitId bundleUnit) {
final String bundle = bundleUnit.toString();
CompletableFuture<Optional<String>> owner;
if (isSystemNamespace(bundleUnit.toString())) {
owner = serviceUnitStateChannel.getChannelOwnerAsync();
} else {
owner = serviceUnitStateChannel.getOwnerAsync(bundle);
}
return owner;
There was a problem hiding this comment.
Yes, adding another namespace will be better. Also, we shouldn't let users delete the system namespace, right?
There was a problem hiding this comment.
I dont think users need to delete system topics.
There was a problem hiding this comment.
Ok, then checkOwnershipPresentAsync will not be used for the system topics.
There was a problem hiding this comment.
Plz throw NotSupportedOperationException if the input bundle's namespace is the system namespace .
PIP: #16691
Motivation
Raising a PR to implement #16691.
We need to support delete namespace bundle admin API.
Modifications
Documentation
docdoc-requireddoc-not-neededdoc-complete