Committed by
Gerrit Code Review
Fix ML_SYNC_ON_FIELD_TO_GUARD_CHANGING_THAT_FIELD issue
Change-Id: I367514c698e825ccf6fda63104e2a51f51c190c4
Showing
1 changed file
with
45 additions
and
28 deletions
| ... | @@ -32,6 +32,8 @@ import java.util.HashMap; | ... | @@ -32,6 +32,8 @@ import java.util.HashMap; |
| 32 | import java.util.HashSet; | 32 | import java.util.HashSet; |
| 33 | import java.util.List; | 33 | import java.util.List; |
| 34 | import java.util.Set; | 34 | import java.util.Set; |
| 35 | +import java.util.concurrent.locks.Lock; | ||
| 36 | +import java.util.concurrent.locks.ReentrantLock; | ||
| 35 | 37 | ||
| 36 | import static com.google.common.base.Preconditions.checkNotNull; | 38 | import static com.google.common.base.Preconditions.checkNotNull; |
| 37 | 39 | ||
| ... | @@ -45,7 +47,8 @@ public class DefaultRoutingHandler { | ... | @@ -45,7 +47,8 @@ public class DefaultRoutingHandler { |
| 45 | private HashMap<DeviceId, ECMPShortestPathGraph> currentEcmpSpgMap; | 47 | private HashMap<DeviceId, ECMPShortestPathGraph> currentEcmpSpgMap; |
| 46 | private HashMap<DeviceId, ECMPShortestPathGraph> updatedEcmpSpgMap; | 48 | private HashMap<DeviceId, ECMPShortestPathGraph> updatedEcmpSpgMap; |
| 47 | private DeviceConfiguration config; | 49 | private DeviceConfiguration config; |
| 48 | - private Status populationStatus; | 50 | + private final Lock statusLock = new ReentrantLock(); |
| 51 | + private volatile Status populationStatus; | ||
| 49 | 52 | ||
| 50 | /** | 53 | /** |
| 51 | * Represents the default routing population status. | 54 | * Represents the default routing population status. |
| ... | @@ -86,35 +89,40 @@ public class DefaultRoutingHandler { | ... | @@ -86,35 +89,40 @@ public class DefaultRoutingHandler { |
| 86 | */ | 89 | */ |
| 87 | public boolean populateAllRoutingRules() { | 90 | public boolean populateAllRoutingRules() { |
| 88 | 91 | ||
| 89 | - populationStatus = Status.STARTED; | 92 | + statusLock.lock(); |
| 90 | - rulePopulator.resetCounter(); | 93 | + try { |
| 91 | - log.info("Starts to populate routing rules"); | 94 | + populationStatus = Status.STARTED; |
| 92 | - log.debug("populateAllRoutingRules: populationStatus is STARTED"); | 95 | + rulePopulator.resetCounter(); |
| 96 | + log.info("Starts to populate routing rules"); | ||
| 97 | + log.debug("populateAllRoutingRules: populationStatus is STARTED"); | ||
| 93 | 98 | ||
| 94 | - for (Device sw : srManager.deviceService.getDevices()) { | 99 | + for (Device sw : srManager.deviceService.getDevices()) { |
| 95 | - if (srManager.mastershipService.getLocalRole(sw.id()) != MastershipRole.MASTER) { | 100 | + if (srManager.mastershipService.getLocalRole(sw.id()) != MastershipRole.MASTER) { |
| 96 | - log.debug("populateAllRoutingRules: skipping device {}...we are not master", | 101 | + log.debug("populateAllRoutingRules: skipping device {}...we are not master", |
| 97 | - sw.id()); | 102 | + sw.id()); |
| 98 | - continue; | 103 | + continue; |
| 99 | - } | 104 | + } |
| 100 | 105 | ||
| 101 | - ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(sw.id(), srManager); | 106 | + ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(sw.id(), srManager); |
| 102 | - if (!populateEcmpRoutingRules(sw.id(), ecmpSpg)) { | 107 | + if (!populateEcmpRoutingRules(sw.id(), ecmpSpg)) { |
| 103 | - log.debug("populateAllRoutingRules: populationStatus is ABORTED"); | 108 | + log.debug("populateAllRoutingRules: populationStatus is ABORTED"); |
| 104 | - populationStatus = Status.ABORTED; | 109 | + populationStatus = Status.ABORTED; |
| 105 | - log.debug("Abort routing rule population"); | 110 | + log.debug("Abort routing rule population"); |
| 106 | - return false; | 111 | + return false; |
| 112 | + } | ||
| 113 | + currentEcmpSpgMap.put(sw.id(), ecmpSpg); | ||
| 114 | + | ||
| 115 | + // TODO: Set adjacency routing rule for all switches | ||
| 107 | } | 116 | } |
| 108 | - currentEcmpSpgMap.put(sw.id(), ecmpSpg); | ||
| 109 | 117 | ||
| 110 | - // TODO: Set adjacency routing rule for all switches | 118 | + log.debug("populateAllRoutingRules: populationStatus is SUCCEEDED"); |
| 119 | + populationStatus = Status.SUCCEEDED; | ||
| 120 | + log.info("Completes routing rule population. Total # of rules pushed : {}", | ||
| 121 | + rulePopulator.getCounter()); | ||
| 122 | + return true; | ||
| 123 | + } finally { | ||
| 124 | + statusLock.unlock(); | ||
| 111 | } | 125 | } |
| 112 | - | ||
| 113 | - log.debug("populateAllRoutingRules: populationStatus is SUCCEEDED"); | ||
| 114 | - populationStatus = Status.SUCCEEDED; | ||
| 115 | - log.info("Completes routing rule population. Total # of rules pushed : {}", | ||
| 116 | - rulePopulator.getCounter()); | ||
| 117 | - return true; | ||
| 118 | } | 126 | } |
| 119 | 127 | ||
| 120 | /** | 128 | /** |
| ... | @@ -127,7 +135,8 @@ public class DefaultRoutingHandler { | ... | @@ -127,7 +135,8 @@ public class DefaultRoutingHandler { |
| 127 | */ | 135 | */ |
| 128 | public boolean populateRoutingRulesForLinkStatusChange(Link linkFail) { | 136 | public boolean populateRoutingRulesForLinkStatusChange(Link linkFail) { |
| 129 | 137 | ||
| 130 | - synchronized (populationStatus) { | 138 | + statusLock.lock(); |
| 139 | + try { | ||
| 131 | 140 | ||
| 132 | if (populationStatus == Status.STARTED) { | 141 | if (populationStatus == Status.STARTED) { |
| 133 | log.warn("Previous rule population is not finished."); | 142 | log.warn("Previous rule population is not finished."); |
| ... | @@ -179,6 +188,8 @@ public class DefaultRoutingHandler { | ... | @@ -179,6 +188,8 @@ public class DefaultRoutingHandler { |
| 179 | log.warn("Failed to repopulate the rules."); | 188 | log.warn("Failed to repopulate the rules."); |
| 180 | return false; | 189 | return false; |
| 181 | } | 190 | } |
| 191 | + } finally { | ||
| 192 | + statusLock.unlock(); | ||
| 182 | } | 193 | } |
| 183 | } | 194 | } |
| 184 | 195 | ||
| ... | @@ -504,7 +515,8 @@ public class DefaultRoutingHandler { | ... | @@ -504,7 +515,8 @@ public class DefaultRoutingHandler { |
| 504 | * ABORTED status when any groups required for flows is not set yet. | 515 | * ABORTED status when any groups required for flows is not set yet. |
| 505 | */ | 516 | */ |
| 506 | public void startPopulationProcess() { | 517 | public void startPopulationProcess() { |
| 507 | - synchronized (populationStatus) { | 518 | + statusLock.lock(); |
| 519 | + try { | ||
| 508 | if (populationStatus == Status.IDLE | 520 | if (populationStatus == Status.IDLE |
| 509 | || populationStatus == Status.SUCCEEDED | 521 | || populationStatus == Status.SUCCEEDED |
| 510 | || populationStatus == Status.ABORTED) { | 522 | || populationStatus == Status.ABORTED) { |
| ... | @@ -514,6 +526,8 @@ public class DefaultRoutingHandler { | ... | @@ -514,6 +526,8 @@ public class DefaultRoutingHandler { |
| 514 | log.warn("Not initiating startPopulationProcess as populationStatus is {}", | 526 | log.warn("Not initiating startPopulationProcess as populationStatus is {}", |
| 515 | populationStatus); | 527 | populationStatus); |
| 516 | } | 528 | } |
| 529 | + } finally { | ||
| 530 | + statusLock.unlock(); | ||
| 517 | } | 531 | } |
| 518 | } | 532 | } |
| 519 | 533 | ||
| ... | @@ -522,13 +536,16 @@ public class DefaultRoutingHandler { | ... | @@ -522,13 +536,16 @@ public class DefaultRoutingHandler { |
| 522 | * Mostly the process is aborted when the groups required are not set yet. | 536 | * Mostly the process is aborted when the groups required are not set yet. |
| 523 | */ | 537 | */ |
| 524 | public void resumePopulationProcess() { | 538 | public void resumePopulationProcess() { |
| 525 | - synchronized (populationStatus) { | 539 | + statusLock.lock(); |
| 540 | + try { | ||
| 526 | if (populationStatus == Status.ABORTED) { | 541 | if (populationStatus == Status.ABORTED) { |
| 527 | populationStatus = Status.STARTED; | 542 | populationStatus = Status.STARTED; |
| 528 | // TODO: we need to restart from the point aborted instead of | 543 | // TODO: we need to restart from the point aborted instead of |
| 529 | // restarting. | 544 | // restarting. |
| 530 | populateAllRoutingRules(); | 545 | populateAllRoutingRules(); |
| 531 | } | 546 | } |
| 547 | + } finally { | ||
| 548 | + statusLock.unlock(); | ||
| 532 | } | 549 | } |
| 533 | } | 550 | } |
| 534 | } | 551 | } | ... | ... |
-
Please register or login to post a comment