ONOS-1798: More appropriate method name + better pre-condition checks
Change-Id: I5d277967c22b9db48097288ea072cbc4d1c5a2a8
diff --git a/core/api/src/main/java/org/onosproject/cluster/LeadershipService.java b/core/api/src/main/java/org/onosproject/cluster/LeadershipService.java
index c228d32..9b94056 100644
--- a/core/api/src/main/java/org/onosproject/cluster/LeadershipService.java
+++ b/core/api/src/main/java/org/onosproject/cluster/LeadershipService.java
@@ -74,7 +74,8 @@
* potentially become the leader again if and when it becomes the highest
* priority candidate
* <p>
- * If the local nodeId is not the leader, this method will be a noop.
+ * If the local nodeId is not the leader, this method will make no changes and
+ * simply return false.
*
* @param path topic for which this controller node should give up leadership
* @return true if this node stepped down from leadership, false otherwise
diff --git a/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DistributedLeadershipManager.java b/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DistributedLeadershipManager.java
index 74dae01..ed34928 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DistributedLeadershipManager.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/consistent/impl/DistributedLeadershipManager.java
@@ -271,7 +271,7 @@
@Override
public boolean stepdown(String path) {
- if (!activeTopics.contains(path)) {
+ if (!activeTopics.contains(path) || !Objects.equals(localNodeId, getLeader(path))) {
return false;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/ConsistentDeviceMastershipStore.java b/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/ConsistentDeviceMastershipStore.java
index d7d2bc0..8cd6b59 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/ConsistentDeviceMastershipStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/mastership/impl/ConsistentDeviceMastershipStore.java
@@ -93,7 +93,7 @@
new MessageSubject("mastership-store-device-role-query");
private static final MessageSubject ROLE_RELINQUISH_SUBJECT =
new MessageSubject("mastership-store-device-role-relinquish");
- private static final MessageSubject MASTERSHIP_RELINQUISH_SUBJECT =
+ private static final MessageSubject TRANSITION_FROM_MASTER_TO_STANDBY_SUBJECT =
new MessageSubject("mastership-store-device-mastership-relinquish");
private static final Pattern DEVICE_MASTERSHIP_TOPIC_PATTERN =
@@ -128,9 +128,9 @@
clusterCommunicator.addSubscriber(ROLE_RELINQUISH_SUBJECT,
new RoleRelinquishHandler(),
messageHandlingExecutor);
- clusterCommunicator.addSubscriber(MASTERSHIP_RELINQUISH_SUBJECT,
+ clusterCommunicator.addSubscriber(TRANSITION_FROM_MASTER_TO_STANDBY_SUBJECT,
SERIALIZER::decode,
- this::relinquishMastership,
+ this::transitionFromMasterToStandby,
SERIALIZER::encode,
messageHandlingExecutor);
localNodeId = clusterService.getLocalNode().id();
@@ -143,7 +143,7 @@
public void deactivate() {
clusterCommunicator.removeSubscriber(ROLE_QUERY_SUBJECT);
clusterCommunicator.removeSubscriber(ROLE_RELINQUISH_SUBJECT);
- clusterCommunicator.removeSubscriber(MASTERSHIP_RELINQUISH_SUBJECT);
+ clusterCommunicator.removeSubscriber(TRANSITION_FROM_MASTER_TO_STANDBY_SUBJECT);
messageHandlingExecutor.shutdown();
leadershipService.removeListener(leadershipEventListener);
@@ -256,7 +256,7 @@
return null;
}
if (leadershipService.makeTopCandidate(leadershipTopic, nodeId)) {
- return relinquishMastership(deviceId);
+ return transitionFromMasterToStandby(deviceId);
} else {
log.warn("Failed to promote {} to mastership for {}", nodeId, deviceId);
}
@@ -282,9 +282,9 @@
if (!nodeId.equals(currentMaster)) {
return null;
}
- // FIXME: This can becomes the master again unless it
- // is demoted to the end of candidates list.
- return relinquishMastership(deviceId);
+ // FIXME: This can become the master again unless it
+ // is first demoted to the end of candidates list.
+ return transitionFromMasterToStandby(deviceId);
}
@Override
@@ -309,14 +309,11 @@
}
String leadershipTopic = createDeviceMastershipTopic(deviceId);
- Leadership currentLeadership = leadershipService.getLeadership(leadershipTopic);
+ NodeId currentLeader = leadershipService.getLeader(leadershipTopic);
- MastershipEvent.Type eventType = null;
- if (currentLeadership != null && currentLeadership.leader().equals(localNodeId)) {
- eventType = MastershipEvent.Type.MASTER_CHANGED;
- } else {
- eventType = MastershipEvent.Type.BACKUPS_CHANGED;
- }
+ MastershipEvent.Type eventType = Objects.equal(currentLeader, localNodeId)
+ ? MastershipEvent.Type.MASTER_CHANGED
+ : MastershipEvent.Type.BACKUPS_CHANGED;
connectedDevices.remove(deviceId);
leadershipService.withdraw(leadershipTopic);
@@ -324,7 +321,7 @@
return new MastershipEvent(eventType, deviceId, getNodes(deviceId));
}
- private MastershipEvent relinquishMastership(DeviceId deviceId) {
+ private MastershipEvent transitionFromMasterToStandby(DeviceId deviceId) {
checkArgument(deviceId != null, DEVICE_ID_NULL);
NodeId currentMaster = getMaster(deviceId);
@@ -337,22 +334,14 @@
+ "mastership for device {} to {}", deviceId, currentMaster);
return futureGetOrElse(clusterCommunicator.sendAndReceive(
deviceId,
- MASTERSHIP_RELINQUISH_SUBJECT,
+ TRANSITION_FROM_MASTER_TO_STANDBY_SUBJECT,
SERIALIZER::encode,
SERIALIZER::decode,
currentMaster), null);
}
- String leadershipTopic = createDeviceMastershipTopic(deviceId);
- Leadership currentLeadership = leadershipService.getLeadership(leadershipTopic);
-
- MastershipEvent.Type eventType = null;
- if (currentLeadership != null && currentLeadership.leader().equals(localNodeId)) {
- eventType = MastershipEvent.Type.MASTER_CHANGED;
- }
-
- return leadershipService.stepdown(leadershipTopic)
- ? new MastershipEvent(eventType, deviceId, getNodes(deviceId)) : null;
+ return leadershipService.stepdown(createDeviceMastershipTopic(deviceId))
+ ? new MastershipEvent(MastershipEvent.Type.MASTER_CHANGED, deviceId, getNodes(deviceId)) : null;
}
private class RoleQueryHandler implements ClusterMessageHandler {