You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Demogorgon314 (via GitHub)" <gi...@apache.org> on 2023/08/29 04:38:57 UTC
[GitHub] [pulsar] Demogorgon314 opened a new pull request, #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
Demogorgon314 opened a new pull request, #21083:
URL: https://github.com/apache/pulsar/pull/21083
### Motivation
Currently, when `ServiceUnitStateChannelImpl#doCleanup(String broker)` method gets called, it always uses the current broker address to build the heartbeat namespace. We should build the namespace by the parameter.
### Modifications
* Cleanup correctly heartbeat bundle ownership when handling broker deletion event
* Add unit test to verify.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on pull request #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
Posted by "Demogorgon314 (via GitHub)" <gi...@apache.org>.
Demogorgon314 commented on PR #21083:
URL: https://github.com/apache/pulsar/pull/21083#issuecomment-1702056862
For `HeartbeatNamespace` the `advertisedAddress + config.getWebServicePort().isPresent()?config.getWebServicePort().get() : config.getWebServicePortTls().orElseThrow()` is same as `LookupServiceAddress`
```
public static NamespaceName getHeartbeatNamespaceV2(String host, ServiceConfiguration config) {
Integer port = null;
if (config.getWebServicePort().isPresent()) {
port = config.getWebServicePort().get();
} else if (config.getWebServicePortTls().isPresent()) {
port = config.getWebServicePortTls().get();
}
return NamespaceName.get(String.format(HEARTBEAT_NAMESPACE_FMT_V2, host, port));
}
```
```
public String getLookupServiceAddress() {
return String.format("%s:%s", advertisedAddress, config.getWebServicePort().isPresent()
? config.getWebServicePort().get()
: config.getWebServicePortTls().orElseThrow());
}
```
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 closed pull request #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
Posted by "Demogorgon314 (via GitHub)" <gi...@apache.org>.
Demogorgon314 closed pull request #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
URL: https://github.com/apache/pulsar/pull/21083
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui merged PR #21083:
URL: https://github.com/apache/pulsar/pull/21083
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Demogorgon314 commented on a diff in pull request #21083: [fix][broker] Cleanup correctly heartbeat bundle ownership when handling broker deletion event
Posted by "Demogorgon314 (via GitHub)" <gi...@apache.org>.
Demogorgon314 commented on code in PR #21083:
URL: https://github.com/apache/pulsar/pull/21083#discussion_r1312477267
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -1214,10 +1217,9 @@ private synchronized void doCleanup(String broker) {
int orphanServiceUnitCleanupCnt = 0;
long totalCleanupErrorCntStart = totalCleanupErrorCnt.get();
String heartbeatNamespace =
- NamespaceService.getHeartbeatNamespace(pulsar.getAdvertisedAddress(), pulsar.getConfiguration())
- .toString();
- String heartbeatNamespaceV2 = NamespaceService.getHeartbeatNamespaceV2(pulsar.getAdvertisedAddress(),
- pulsar.getConfiguration()).toString();
+ NamespaceName.get(String.format(HEARTBEAT_NAMESPACE_FMT, config.getClusterName(), broker)).toString();
Review Comment:
> Why does lookupServiceAddress need to be used here? (can you give us some example cases?)
@heesung-sn Because we need to build heartbeat namespace for the needed cleanup broker.
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org