You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/27 09:27:13 UTC
[GitHub] [pulsar] Nicklee007 opened a new pull request, #16239: [fix][broker]fix npe when invoke replaceBookie.
Nicklee007 opened a new pull request, #16239:
URL: https://github.com/apache/pulsar/pull/16239
### Motivation
Fix the npe when the `ISOLATION_BOOKIE_GROUPS` is `"" `or missed which will produce in follow two case and can be reproduce by the unit test in this PR also.
**The npe point**
https://github.com/apache/pulsar/blob/eeb22ba9631d86528bbca8f1825feff7e70272d2/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java#L220
https://github.com/apache/pulsar/blob/eeb22ba9631d86528bbca8f1825feff7e70272d2/pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java#L247
### Modifications
**Case 1:**
When we set `strictBookieAffinityEnabled = true` and if some namespaces not set `bookieAffinityGroup` , and then those namespaces will be set `ISOLATION_BOOKIE_GROUPS` as `""` by default, which will cause npe when invoke `replaceBookie`.
https://github.com/apache/pulsar/blob/eeb22ba9631d86528bbca8f1825feff7e70272d2/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1513
https://github.com/apache/pulsar/blob/eeb22ba9631d86528bbca8f1825feff7e70272d2/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L1529-L1534
**Case 2:**
When the `bookieAffinityGroup` is not null and the `ISOLATION_BOOKIE_GROUPS` is null or "" also cause npe
*Describe the modifications you've done.*
set a default value when `primaryIsolationGroupString` is empty.
- [X] `doc-not-needed`
--
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] gaozhangmin merged pull request #16239: [fix][broker]fix npe when invoke replaceBookie.
Posted by GitBox <gi...@apache.org>.
gaozhangmin merged PR #16239:
URL: https://github.com/apache/pulsar/pull/16239
--
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] Nicklee007 commented on a diff in pull request #16239: [fix][broker]fix npe when invoke replaceBookie.
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on code in PR #16239:
URL: https://github.com/apache/pulsar/pull/16239#discussion_r907283014
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java:
##########
@@ -206,9 +206,13 @@ private static Pair<Set<String>, Set<String>> getIsolationGroup(
castToString(properties.getOrDefault(SECONDARY_ISOLATION_BOOKIE_GROUPS, ""));
if (!primaryIsolationGroupString.isEmpty()) {
pair.setLeft(new HashSet(Arrays.asList(primaryIsolationGroupString.split(","))));
+ } else {
+ pair.setLeft(new HashSet());
}
if (!secondaryIsolationGroupString.isEmpty()) {
pair.setRight(new HashSet(Arrays.asList(secondaryIsolationGroupString.split(","))));
+ } else {
+ pair.setRight(new HashSet());
Review Comment:
Has changed the default value, PTAL.
--
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 commented on a diff in pull request #16239: [fix][broker]fix npe when invoke replaceBookie.
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #16239:
URL: https://github.com/apache/pulsar/pull/16239#discussion_r907261919
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java:
##########
@@ -206,9 +206,13 @@ private static Pair<Set<String>, Set<String>> getIsolationGroup(
castToString(properties.getOrDefault(SECONDARY_ISOLATION_BOOKIE_GROUPS, ""));
if (!primaryIsolationGroupString.isEmpty()) {
pair.setLeft(new HashSet(Arrays.asList(primaryIsolationGroupString.split(","))));
+ } else {
+ pair.setLeft(new HashSet());
Review Comment:
```suggestion
pair.setLeft(Collections.emptySet());
```
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/IsolatedBookieEnsemblePlacementPolicy.java:
##########
@@ -206,9 +206,13 @@ private static Pair<Set<String>, Set<String>> getIsolationGroup(
castToString(properties.getOrDefault(SECONDARY_ISOLATION_BOOKIE_GROUPS, ""));
if (!primaryIsolationGroupString.isEmpty()) {
pair.setLeft(new HashSet(Arrays.asList(primaryIsolationGroupString.split(","))));
+ } else {
+ pair.setLeft(new HashSet());
}
if (!secondaryIsolationGroupString.isEmpty()) {
pair.setRight(new HashSet(Arrays.asList(secondaryIsolationGroupString.split(","))));
+ } else {
+ pair.setRight(new HashSet());
Review Comment:
```suggestion
pair.setLeft(Collections.emptySet());
```
--
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] Nicklee007 commented on pull request #16239: [fix][broker]fix npe when invoke replaceBookie.
Posted by GitBox <gi...@apache.org>.
Nicklee007 commented on PR #16239:
URL: https://github.com/apache/pulsar/pull/16239#issuecomment-1168598187
/pulsarbot run-failure-checks
--
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