-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix][broker] Handle heartbeat namespace in ExtensibleLoadManager #20551
[fix][broker] Handle heartbeat namespace in ExtensibleLoadManager #20551
Conversation
115ca33
to
7aa18bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm. Great work!
...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java
Outdated
Show resolved
Hide resolved
...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java
Outdated
Show resolved
Hide resolved
} | ||
return CompletableFuture.completedFuture(brokerLookupData); | ||
})); | ||
private CompletableFuture<Optional<BrokerLookupData>> getBrokerLookupData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the same idea with @mattisonchao
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The serviceUnitStateChannel.getChannelOwnerAsync()
will return Optional
, so we can change this method.
a213c69
to
972d640
Compare
PIP: #16691
Motivation
The heartbeat namespace bundle requires acquiring ownership when broker starts,
however, the
ExtensibleLoadManager
does not handle this case,we should register the heartbeat namespace to let the health check work.
Modifications
tryAcquiringOwnership
method to acquire the local broker ownershipDocumentation
doc
doc-required
doc-not-needed
doc-complete