Yuta HIGUCHI

allow null Master in MastershipTerm

Change-Id: I840354eb6d0b5a1bac91887a41626c33c49d592c
...@@ -55,13 +55,8 @@ public final class MastershipTerm { ...@@ -55,13 +55,8 @@ public final class MastershipTerm {
55 } 55 }
56 if (other instanceof MastershipTerm) { 56 if (other instanceof MastershipTerm) {
57 MastershipTerm that = (MastershipTerm) other; 57 MastershipTerm that = (MastershipTerm) other;
58 - if (!this.master.equals(that.master)) { 58 + return Objects.equals(this.master, that.master) &&
59 - return false; 59 + Objects.equals(this.termNumber, that.termNumber);
60 - }
61 - if (this.termNumber != that.termNumber) {
62 - return false;
63 - }
64 - return true;
65 } 60 }
66 return false; 61 return false;
67 } 62 }
......
...@@ -257,7 +257,8 @@ public class DeviceManager ...@@ -257,7 +257,8 @@ public class DeviceManager
257 MastershipTerm term = mastershipService.requestTermService() 257 MastershipTerm term = mastershipService.requestTermService()
258 .getMastershipTerm(deviceId); 258 .getMastershipTerm(deviceId);
259 259
260 - if (!term.master().equals(clusterService.getLocalNode().id())) { 260 + final NodeId myNodeId = clusterService.getLocalNode().id();
261 + if (!myNodeId.equals(term.master())) {
261 // lost mastership after requestRole told this instance was MASTER. 262 // lost mastership after requestRole told this instance was MASTER.
262 log.info("lost mastership before getting term info."); 263 log.info("lost mastership before getting term info.");
263 return; 264 return;
......
...@@ -139,14 +139,14 @@ public class MastershipManagerTest { ...@@ -139,14 +139,14 @@ public class MastershipManagerTest {
139 public void termService() { 139 public void termService() {
140 MastershipTermService ts = mgr.requestTermService(); 140 MastershipTermService ts = mgr.requestTermService();
141 141
142 - //term = 0 for both 142 + //term = 1 for both
143 mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); 143 mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
144 - assertEquals("inconsistent term: ", 0, ts.getMastershipTerm(DEV_MASTER).termNumber()); 144 + assertEquals("inconsistent term: ", 1, ts.getMastershipTerm(DEV_MASTER).termNumber());
145 145
146 - //hand devices to NID_LOCAL and back: term = 2 146 + //hand devices to NID_LOCAL and back: term = 1 + 2
147 mgr.setRole(NID_OTHER, DEV_MASTER, MASTER); 147 mgr.setRole(NID_OTHER, DEV_MASTER, MASTER);
148 mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER); 148 mgr.setRole(NID_LOCAL, DEV_MASTER, MASTER);
149 - assertEquals("inconsistent terms: ", 2, ts.getMastershipTerm(DEV_MASTER).termNumber()); 149 + assertEquals("inconsistent terms: ", 3, ts.getMastershipTerm(DEV_MASTER).termNumber());
150 } 150 }
151 151
152 private final class TestClusterService implements ClusterService { 152 private final class TestClusterService implements ClusterService {
......
...@@ -60,8 +60,10 @@ public class DistributedMastershipStore ...@@ -60,8 +60,10 @@ public class DistributedMastershipStore
60 extends AbstractHazelcastStore<MastershipEvent, MastershipStoreDelegate> 60 extends AbstractHazelcastStore<MastershipEvent, MastershipStoreDelegate>
61 implements MastershipStore { 61 implements MastershipStore {
62 62
63 + //term number representing that master has never been chosen yet
64 + private static final Integer NOTHING = 0;
63 //initial term/TTL value 65 //initial term/TTL value
64 - private static final Integer INIT = 0; 66 + private static final Integer INIT = 1;
65 67
66 //device to node roles 68 //device to node roles
67 protected SMap<DeviceId, RoleValue> roleMap; 69 protected SMap<DeviceId, RoleValue> roleMap;
...@@ -256,8 +258,8 @@ implements MastershipStore { ...@@ -256,8 +258,8 @@ implements MastershipStore {
256 RoleValue rv = getRoleValue(deviceId); 258 RoleValue rv = getRoleValue(deviceId);
257 final Integer term = terms.get(deviceId); 259 final Integer term = terms.get(deviceId);
258 final NodeId master = rv.get(MASTER); 260 final NodeId master = rv.get(MASTER);
259 - if ((master == null) || (term == null)) { 261 + if (term == null) {
260 - return null; 262 + return MastershipTerm.of(null, NOTHING);
261 } 263 }
262 return MastershipTerm.of(master, term); 264 return MastershipTerm.of(master, term);
263 } finally { 265 } finally {
......
...@@ -149,7 +149,7 @@ public class DistributedMastershipStoreTest { ...@@ -149,7 +149,7 @@ public class DistributedMastershipStoreTest {
149 assertEquals("wrong role for NONE:", MASTER, dms.requestRole(DID1)); 149 assertEquals("wrong role for NONE:", MASTER, dms.requestRole(DID1));
150 assertTrue("wrong state for store:", !dms.terms.isEmpty()); 150 assertTrue("wrong state for store:", !dms.terms.isEmpty());
151 assertEquals("wrong term", 151 assertEquals("wrong term",
152 - MastershipTerm.of(N1, 0), dms.getTermFor(DID1)); 152 + MastershipTerm.of(N1, 1), dms.getTermFor(DID1));
153 153
154 //CN2 now local. DID2 has N1 as MASTER so N2 is STANDBY 154 //CN2 now local. DID2 has N1 as MASTER so N2 is STANDBY
155 testStore.setCurrent(CN2); 155 testStore.setCurrent(CN2);
...@@ -172,15 +172,15 @@ public class DistributedMastershipStoreTest { ...@@ -172,15 +172,15 @@ public class DistributedMastershipStoreTest {
172 //switch over to N2 172 //switch over to N2
173 assertEquals("wrong event:", Type.MASTER_CHANGED, dms.setMaster(N2, DID1).type()); 173 assertEquals("wrong event:", Type.MASTER_CHANGED, dms.setMaster(N2, DID1).type());
174 System.out.println(dms.getTermFor(DID1).master() + ":" + dms.getTermFor(DID1).termNumber()); 174 System.out.println(dms.getTermFor(DID1).master() + ":" + dms.getTermFor(DID1).termNumber());
175 - assertEquals("wrong term", MastershipTerm.of(N2, 1), dms.getTermFor(DID1)); 175 + assertEquals("wrong term", MastershipTerm.of(N2, 2), dms.getTermFor(DID1));
176 176
177 //orphan switch - should be rare case 177 //orphan switch - should be rare case
178 assertEquals("wrong event:", Type.MASTER_CHANGED, dms.setMaster(N2, DID2).type()); 178 assertEquals("wrong event:", Type.MASTER_CHANGED, dms.setMaster(N2, DID2).type());
179 - assertEquals("wrong term", MastershipTerm.of(N2, 0), dms.getTermFor(DID2)); 179 + assertEquals("wrong term", MastershipTerm.of(N2, 1), dms.getTermFor(DID2));
180 //disconnect and reconnect - sign of failing re-election or single-instance channel 180 //disconnect and reconnect - sign of failing re-election or single-instance channel
181 dms.roleMap.clear(); 181 dms.roleMap.clear();
182 dms.setMaster(N2, DID2); 182 dms.setMaster(N2, DID2);
183 - assertEquals("wrong term", MastershipTerm.of(N2, 1), dms.getTermFor(DID2)); 183 + assertEquals("wrong term", MastershipTerm.of(N2, 2), dms.getTermFor(DID2));
184 } 184 }
185 185
186 @Test 186 @Test
......
...@@ -38,15 +38,14 @@ public class MastershipTermSerializer extends Serializer<MastershipTerm> { ...@@ -38,15 +38,14 @@ public class MastershipTermSerializer extends Serializer<MastershipTerm> {
38 38
39 @Override 39 @Override
40 public MastershipTerm read(Kryo kryo, Input input, Class<MastershipTerm> type) { 40 public MastershipTerm read(Kryo kryo, Input input, Class<MastershipTerm> type) {
41 - final NodeId node = new NodeId(input.readString()); 41 + final NodeId node = (NodeId) kryo.readClassAndObject(input);
42 final int term = input.readInt(); 42 final int term = input.readInt();
43 return MastershipTerm.of(node, term); 43 return MastershipTerm.of(node, term);
44 } 44 }
45 45
46 @Override 46 @Override
47 public void write(Kryo kryo, Output output, MastershipTerm object) { 47 public void write(Kryo kryo, Output output, MastershipTerm object) {
48 - output.writeString(object.master().toString()); 48 + kryo.writeClassAndObject(output, object.master());
49 output.writeInt(object.termNumber()); 49 output.writeInt(object.termNumber());
50 } 50 }
51 -
52 } 51 }
......
...@@ -201,6 +201,7 @@ public class KryoSerializerTest { ...@@ -201,6 +201,7 @@ public class KryoSerializerTest {
201 @Test 201 @Test
202 public void testMastershipTerm() { 202 public void testMastershipTerm() {
203 testSerialized(MastershipTerm.of(new NodeId("foo"), 2)); 203 testSerialized(MastershipTerm.of(new NodeId("foo"), 2));
204 + testSerialized(MastershipTerm.of(null, 0));
204 } 205 }
205 206
206 @Test 207 @Test
......
...@@ -62,6 +62,9 @@ public class SimpleMastershipStore ...@@ -62,6 +62,9 @@ public class SimpleMastershipStore
62 62
63 public static final IpAddress LOCALHOST = IpAddress.valueOf("127.0.0.1"); 63 public static final IpAddress LOCALHOST = IpAddress.valueOf("127.0.0.1");
64 64
65 + private static final int NOTHING = 0;
66 + private static final int INIT = 1;
67 +
65 private ControllerNode instance = 68 private ControllerNode instance =
66 new DefaultControllerNode(new NodeId("local"), LOCALHOST); 69 new DefaultControllerNode(new NodeId("local"), LOCALHOST);
67 70
...@@ -97,7 +100,7 @@ public class SimpleMastershipStore ...@@ -97,7 +100,7 @@ public class SimpleMastershipStore
97 break; 100 break;
98 case NONE: 101 case NONE:
99 masterMap.put(deviceId, nodeId); 102 masterMap.put(deviceId, nodeId);
100 - termMap.put(deviceId, new AtomicInteger()); 103 + termMap.put(deviceId, new AtomicInteger(INIT));
101 backups.add(nodeId); 104 backups.add(nodeId);
102 break; 105 break;
103 default: 106 default:
...@@ -149,7 +152,7 @@ public class SimpleMastershipStore ...@@ -149,7 +152,7 @@ public class SimpleMastershipStore
149 NodeId rel = reelect(node); 152 NodeId rel = reelect(node);
150 if (rel == null) { 153 if (rel == null) {
151 masterMap.put(deviceId, node); 154 masterMap.put(deviceId, node);
152 - termMap.put(deviceId, new AtomicInteger()); 155 + termMap.put(deviceId, new AtomicInteger(INIT));
153 role = MastershipRole.MASTER; 156 role = MastershipRole.MASTER;
154 } 157 }
155 backups.add(node); 158 backups.add(node);
...@@ -159,7 +162,7 @@ public class SimpleMastershipStore ...@@ -159,7 +162,7 @@ public class SimpleMastershipStore
159 //first to get to it, say we are master 162 //first to get to it, say we are master
160 synchronized (this) { 163 synchronized (this) {
161 masterMap.put(deviceId, node); 164 masterMap.put(deviceId, node);
162 - termMap.put(deviceId, new AtomicInteger()); 165 + termMap.put(deviceId, new AtomicInteger(INIT));
163 backups.add(node); 166 backups.add(node);
164 role = MastershipRole.MASTER; 167 role = MastershipRole.MASTER;
165 } 168 }
...@@ -194,9 +197,8 @@ public class SimpleMastershipStore ...@@ -194,9 +197,8 @@ public class SimpleMastershipStore
194 197
195 @Override 198 @Override
196 public MastershipTerm getTermFor(DeviceId deviceId) { 199 public MastershipTerm getTermFor(DeviceId deviceId) {
197 - if ((masterMap.get(deviceId) == null) || 200 + if ((termMap.get(deviceId) == null)) {
198 - (termMap.get(deviceId) == null)) { 201 + return MastershipTerm.of(masterMap.get(deviceId), NOTHING);
199 - return null;
200 } 202 }
201 return MastershipTerm.of( 203 return MastershipTerm.of(
202 masterMap.get(deviceId), termMap.get(deviceId).get()); 204 masterMap.get(deviceId), termMap.get(deviceId).get());
...@@ -220,7 +222,7 @@ public class SimpleMastershipStore ...@@ -220,7 +222,7 @@ public class SimpleMastershipStore
220 case STANDBY: 222 case STANDBY:
221 case NONE: 223 case NONE:
222 if (!termMap.containsKey(deviceId)) { 224 if (!termMap.containsKey(deviceId)) {
223 - termMap.put(deviceId, new AtomicInteger()); 225 + termMap.put(deviceId, new AtomicInteger(INIT));
224 } 226 }
225 backups.add(nodeId); 227 backups.add(nodeId);
226 break; 228 break;
......