eolivelli commented on code in PR #19490:
URL: https://github.com/apache/pulsar/pull/19490#discussion_r1104373474
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateCompactionStrategy.java:
##########
@@ -57,33 +54,45 @@ public boolean shouldKeepLeft(ServiceUnitStateData from,
ServiceUnitStateData to
}
if (checkBrokers) {
- if (prevState == Free && (state == Assigned || state == Owned)) {
- // Free -> Assigned || Owned broker check
- return StringUtils.isBlank(to.broker());
- } else if (prevState == Owned && state == Assigned) {
- // Owned -> Assigned(transfer) broker check
- return !StringUtils.equals(from.broker(), to.sourceBroker())
- || StringUtils.isBlank(to.broker())
- || StringUtils.equals(from.broker(), to.broker());
- } else if (prevState == Assigned && state == Released) {
- // Assigned -> Released(transfer) broker check
- return !StringUtils.equals(from.broker(), to.broker())
- || !StringUtils.equals(from.sourceBroker(),
to.sourceBroker());
- } else if (prevState == Released && state == Owned) {
- // Released -> Owned(transfer) broker check
- return !StringUtils.equals(from.broker(), to.broker())
- || !StringUtils.equals(from.sourceBroker(),
to.sourceBroker());
- } else if (prevState == Assigned && state == Owned) {
- // Assigned -> Owned broker check
- return !StringUtils.equals(from.broker(), to.broker())
- || !StringUtils.equals(from.sourceBroker(),
to.sourceBroker());
- } else if (prevState == Owned && state == Splitting) {
- // Owned -> Splitting broker check
- return !StringUtils.equals(from.broker(), to.broker());
+ switch (prevState) {
+ case Free:
+ switch (state) {
+ case Assigned:
+ case Owned:
+ return isNotBlank(to.sourceBroker());
+ }
+ case Owned:
+ switch (state) {
+ case Assigned:
+ return invalidTransfer(from, to);
+ case Splitting:
+ return !from.broker().equals(to.broker());
+ }
+ case Assigned:
+ switch (state) {
+ case Released:
+ case Owned:
+ return notEquals(from, to);
+ }
+ case Released:
+ switch (state) {
+ case Owned:
+ return notEquals(from, to);
+ }
+ case Splitting:
+ return false;
}
}
-
return false;
}
+ private boolean notEquals(ServiceUnitStateData from, ServiceUnitStateData
to) {
+ return !from.broker().equals(to.broker())
+ || !StringUtils.equals(from.sourceBroker(), to.sourceBroker());
+ }
+
+ private boolean invalidTransfer(ServiceUnitStateData from,
ServiceUnitStateData to) {
+ return !from.broker().equals(to.sourceBroker())
Review Comment:
why aren't we using StringUtils.equals for every comparison ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
