Committed by
Gerrit Code Review
DistributedIntentStore: add sanity check to parking state transition
Change-Id: I2958a5889451a4f7a34146033a801cf89b73a1cc
Showing
1 changed file
with
28 additions
and
13 deletions
| ... | @@ -39,8 +39,10 @@ import org.onlab.onos.store.serializers.KryoSerializer; | ... | @@ -39,8 +39,10 @@ import org.onlab.onos.store.serializers.KryoSerializer; |
| 39 | import org.onlab.util.KryoNamespace; | 39 | import org.onlab.util.KryoNamespace; |
| 40 | import org.slf4j.Logger; | 40 | import org.slf4j.Logger; |
| 41 | 41 | ||
| 42 | +import java.util.EnumSet; | ||
| 42 | import java.util.List; | 43 | import java.util.List; |
| 43 | import java.util.Map; | 44 | import java.util.Map; |
| 45 | +import java.util.Set; | ||
| 44 | import java.util.concurrent.ConcurrentHashMap; | 46 | import java.util.concurrent.ConcurrentHashMap; |
| 45 | 47 | ||
| 46 | import static com.google.common.base.Verify.verify; | 48 | import static com.google.common.base.Verify.verify; |
| ... | @@ -53,6 +55,12 @@ public class DistributedIntentStore | ... | @@ -53,6 +55,12 @@ public class DistributedIntentStore |
| 53 | extends AbstractHazelcastStore<IntentEvent, IntentStoreDelegate> | 55 | extends AbstractHazelcastStore<IntentEvent, IntentStoreDelegate> |
| 54 | implements IntentStore { | 56 | implements IntentStore { |
| 55 | 57 | ||
| 58 | + /** Valid parking state, which can transition to INSTALLED. */ | ||
| 59 | + private static final Set<IntentState> PRE_INSTALLED = EnumSet.of(SUBMITTED, FAILED); | ||
| 60 | + | ||
| 61 | + /** Valid parking state, which can transition to WITHDRAWN. */ | ||
| 62 | + private static final Set<IntentState> PRE_WITHDRAWN = EnumSet.of(INSTALLED, FAILED); | ||
| 63 | + | ||
| 56 | private final Logger log = getLogger(getClass()); | 64 | private final Logger log = getLogger(getClass()); |
| 57 | 65 | ||
| 58 | // Assumption: IntentId will not have synonyms | 66 | // Assumption: IntentId will not have synonyms |
| ... | @@ -158,45 +166,52 @@ public class DistributedIntentStore | ... | @@ -158,45 +166,52 @@ public class DistributedIntentStore |
| 158 | return states.get(id); | 166 | return states.get(id); |
| 159 | } | 167 | } |
| 160 | 168 | ||
| 169 | + | ||
| 161 | @Override | 170 | @Override |
| 162 | public IntentEvent setState(Intent intent, IntentState state) { | 171 | public IntentEvent setState(Intent intent, IntentState state) { |
| 163 | final IntentId id = intent.id(); | 172 | final IntentId id = intent.id(); |
| 164 | IntentEvent.Type type = null; | 173 | IntentEvent.Type type = null; |
| 165 | - IntentState prev = null; | 174 | + final IntentState prevParking; |
| 166 | boolean transientStateChangeOnly = false; | 175 | boolean transientStateChangeOnly = false; |
| 167 | 176 | ||
| 168 | - // TODO: enable sanity checking if Debug enabled, etc. | 177 | + // parking state transition |
| 169 | switch (state) { | 178 | switch (state) { |
| 170 | case SUBMITTED: | 179 | case SUBMITTED: |
| 171 | - prev = states.putIfAbsent(id, SUBMITTED); | 180 | + prevParking = states.putIfAbsent(id, SUBMITTED); |
| 172 | - verify(prev == null, "Illegal state transition attempted from %s to SUBMITTED", prev); | 181 | + verify(prevParking == null, |
| 182 | + "Illegal state transition attempted from %s to SUBMITTED", | ||
| 183 | + prevParking); | ||
| 173 | type = IntentEvent.Type.SUBMITTED; | 184 | type = IntentEvent.Type.SUBMITTED; |
| 174 | break; | 185 | break; |
| 175 | case INSTALLED: | 186 | case INSTALLED: |
| 176 | - // parking state transition | 187 | + prevParking = states.replace(id, INSTALLED); |
| 177 | - prev = states.replace(id, INSTALLED); | 188 | + verify(PRE_INSTALLED.contains(prevParking), |
| 178 | - verify(prev != null, "Illegal state transition attempted from non-SUBMITTED to INSTALLED"); | 189 | + "Illegal state transition attempted from %s to INSTALLED", |
| 190 | + prevParking); | ||
| 179 | type = IntentEvent.Type.INSTALLED; | 191 | type = IntentEvent.Type.INSTALLED; |
| 180 | break; | 192 | break; |
| 181 | case FAILED: | 193 | case FAILED: |
| 182 | - prev = states.replace(id, FAILED); | 194 | + prevParking = states.replace(id, FAILED); |
| 183 | type = IntentEvent.Type.FAILED; | 195 | type = IntentEvent.Type.FAILED; |
| 184 | break; | 196 | break; |
| 185 | case WITHDRAWN: | 197 | case WITHDRAWN: |
| 186 | - prev = states.replace(id, WITHDRAWN); | 198 | + prevParking = states.replace(id, WITHDRAWN); |
| 187 | - verify(prev != null, "Illegal state transition attempted from non-WITHDRAWING to WITHDRAWN"); | 199 | + verify(PRE_WITHDRAWN.contains(prevParking), |
| 200 | + "Illegal state transition attempted from %s to WITHDRAWN", | ||
| 201 | + prevParking); | ||
| 188 | type = IntentEvent.Type.WITHDRAWN; | 202 | type = IntentEvent.Type.WITHDRAWN; |
| 189 | break; | 203 | break; |
| 190 | default: | 204 | default: |
| 191 | transientStateChangeOnly = true; | 205 | transientStateChangeOnly = true; |
| 206 | + prevParking = null; | ||
| 192 | break; | 207 | break; |
| 193 | } | 208 | } |
| 194 | if (!transientStateChangeOnly) { | 209 | if (!transientStateChangeOnly) { |
| 195 | - log.debug("Parking State change: {} {}=>{}", id, prev, state); | 210 | + log.debug("Parking State change: {} {}=>{}", id, prevParking, state); |
| 196 | } | 211 | } |
| 197 | // Update instance local state, which includes non-parking state transition | 212 | // Update instance local state, which includes non-parking state transition |
| 198 | - prev = transientStates.put(id, state); | 213 | + final IntentState prevTransient = transientStates.put(id, state); |
| 199 | - log.debug("Transient State change: {} {}=>{}", id, prev, state); | 214 | + log.debug("Transient State change: {} {}=>{}", id, prevTransient, state); |
| 200 | 215 | ||
| 201 | if (type == null) { | 216 | if (type == null) { |
| 202 | return null; | 217 | return null; | ... | ... |
-
Please register or login to post a comment