You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "eolivelli (via GitHub)" <gi...@apache.org> on 2023/02/13 12:00:52 UTC
[GitHub] [pulsar] eolivelli commented on a diff in pull request #19490: [improve][broker] PIP-192 Use msg property to mark compacted msgs from strategic compaction
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org