DeviceManager fix

- relinquish Mastership on disconnecregardless of store response
- update TODO memos

Change-Id: Idece0326fdc780586801fbeb19004c4e88ac8c92

@@ -203,31 +203,43 @@ public class DeviceManager
             MastershipRole role = mastershipService.requestRoleFor(deviceId);
             MastershipRole role = mastershipService.requestRoleFor(deviceId);
 
 
             if (role != MastershipRole.MASTER) {
             if (role != MastershipRole.MASTER) {
-                // TODO: Do we need to tell the Provider that
-                // I am no longer the MASTER?
+                // TODO: Do we need to explicitly tell the Provider that
+                // this instance is no longer the MASTER? probably not
                 return;
                 return;
             }
             }
 
 
-            // Master:
             MastershipTerm term = mastershipService.requestTermService()
             MastershipTerm term = mastershipService.requestTermService()
                     .getMastershipTerm(deviceId);
                     .getMastershipTerm(deviceId);
             if (!term.master().equals(clusterService.getLocalNode().id())) {
             if (!term.master().equals(clusterService.getLocalNode().id())) {
-                // I've lost mastership after I thought I was MASTER.
+                // lost mastership after requestRole told this instance was MASTER.
                 return;
                 return;
             }
             }
+            // tell clock provider if this instance is the master
             clockProviderService.setMastershipTerm(deviceId, term);
             clockProviderService.setMastershipTerm(deviceId, term);
 
 
             DeviceEvent event = store.createOrUpdateDevice(provider().id(),
             DeviceEvent event = store.createOrUpdateDevice(provider().id(),
                     deviceId, deviceDescription);
                     deviceId, deviceDescription);
 
 
             // If there was a change of any kind, tell the provider
             // If there was a change of any kind, tell the provider
-            // I am the master.
-            // Note: can be null, if mastership was lost.
+            // that this instance is the master.
+            // Note: event can be null, if mastership was lost between
+            // roleRequest and store update calls.
             if (event != null) {
             if (event != null) {
-                // TODO: Check switch reconnected case, is it assured that
-                //       event will not be null?
-
-                // FIXME: 1st argument should be deviceId
+                // TODO: Check switch reconnected case. Is it assured that
+                //       event will never be null?
+                //       Could there be a situation MastershipService told this
+                //       instance is the new Master, but
+                //       event returned from the store is null?
+
+                // TODO: Confirm: Mastership could be lost after requestRole
+                //       and createOrUpdateDevice call.
+                //       In that case STANDBY node can
+                //       claim itself to be master against the Device.
+                //       Will the Node, chosen by the MastershipService, retry
+                //       to get the MASTER role when that happen?
+
+                // FIXME: 1st argument should be deviceId, to allow setting
+                //        certain roles even if the store returned null.
                 provider().roleChanged(event.subject(), role);
                 provider().roleChanged(event.subject(), role);
                 post(event);
                 post(event);
             }
             }
@@ -237,16 +249,22 @@ public class DeviceManager
         public void deviceDisconnected(DeviceId deviceId) {
         public void deviceDisconnected(DeviceId deviceId) {
             checkNotNull(deviceId, DEVICE_ID_NULL);
             checkNotNull(deviceId, DEVICE_ID_NULL);
             checkValidity();
             checkValidity();
+
+            // FIXME: only the MASTER should be marking off-line in normal cases,
+            // but if I was the last STANDBY connection, etc. and no one else
+            // was there to mark the device offline, this instance may need to
+            // temporarily request for Master Role and mark offline.
             if (!mastershipService.getLocalRole(deviceId).equals(MastershipRole.MASTER)) {
             if (!mastershipService.getLocalRole(deviceId).equals(MastershipRole.MASTER)) {
                 log.debug("Device {} disconnected, but I am not the master", deviceId);
                 log.debug("Device {} disconnected, but I am not the master", deviceId);
                 return;
                 return;
             }
             }
             DeviceEvent event = store.markOffline(deviceId);
             DeviceEvent event = store.markOffline(deviceId);
 
 
+            mastershipService.relinquishMastership(deviceId);
+
             //we're no longer capable of mastership.
             //we're no longer capable of mastership.
             if (event != null) {
             if (event != null) {
                 log.info("Device {} disconnected", deviceId);
                 log.info("Device {} disconnected", deviceId);
-                mastershipService.relinquishMastership(deviceId);
                 post(event);
                 post(event);
             }
             }
         }
         }
@@ -286,6 +304,9 @@ public class DeviceManager
             // FIXME: implement response to this notification
             // FIXME: implement response to this notification
             log.warn("Failed to assert role [{}] onto Device {}", role,
             log.warn("Failed to assert role [{}] onto Device {}", role,
                     deviceId);
                     deviceId);
+            if (role == MastershipRole.MASTER) {
+                mastershipService.relinquishMastership(deviceId);
+            }
         }
         }
     }
     }
 
 
@@ -307,7 +328,7 @@ public class DeviceManager
                         .getMastershipTerm(event.subject());
                         .getMastershipTerm(event.subject());
 
 
                 if (term.master().equals(clusterService.getLocalNode().id())) {
                 if (term.master().equals(clusterService.getLocalNode().id())) {
-                    // only set if I am the master
+                    // only set the new term if I am the master
                     clockProviderService.setMastershipTerm(event.subject(), term);
                     clockProviderService.setMastershipTerm(event.subject(), term);
                 }
                 }