From 55481f6f0a6e07fd5a01e37f3ee45ae9ee775414 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 7 Apr 2016 14:33:19 +0530 Subject: [PATCH] IDManager fixes for restart scenario * ID Entry list is added to parent instead of child. During id * allocation, check is added for existing id in parent pool. * Without this fix, each node in cluster used to check only in the * child pool. And when leader changes during restart, child pool * didn't have the corresponding id-entry, resulting in a new * different id. Change-Id: I479da293eb557915c6a20eaaacc364b5e90c2bc1 Signed-off-by: Manu B --- .../org/opendaylight/idmanager/IdManager.java | 112 ++++++++++++------ .../idmanager/test/IdManagerTest.java | 45 +++++-- 2 files changed, 111 insertions(+), 46 deletions(-) diff --git a/idmanager/idmanager-impl/src/main/java/org/opendaylight/idmanager/IdManager.java b/idmanager/idmanager-impl/src/main/java/org/opendaylight/idmanager/IdManager.java index 07f0e3c6..f3251e38 100644 --- a/idmanager/idmanager-impl/src/main/java/org/opendaylight/idmanager/IdManager.java +++ b/idmanager/idmanager-impl/src/main/java/org/opendaylight/idmanager/IdManager.java @@ -12,15 +12,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.LinkedList; import java.util.List; -import java.util.concurrent.ExecutionException; +import java.util.NoSuchElementException; import java.util.concurrent.Future; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.DataChangeListener; -import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; -import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.vpnservice.mdsalutil.MDSALUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.idmanager.rev150403.AllocateIdInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.idmanager.rev150403.AllocateIdOutput; @@ -43,7 +40,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.idmanager.rev150 import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.idmanager.rev150403.released.ids.DelayedIdEntriesBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.vpnservice.lockmanager.rev150819.LockManagerService; import org.opendaylight.yangtools.concepts.ListenerRegistration; -import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.RpcError.ErrorType; import org.opendaylight.yangtools.yang.common.RpcResult; @@ -52,13 +48,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.base.Optional; -import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; public class IdManager implements IdManagerService, AutoCloseable{ private static final Logger LOG = LoggerFactory.getLogger(IdManager.class); - private static final long DEFAULT_IDLE_TIME = 24 * 60 * 60; + private static final long DEFAULT_IDLE_TIME = 24 * 60 * 60; private ListenerRegistration listenerRegistration; private final DataBroker broker; @@ -125,7 +120,7 @@ public class IdManager implements IdManagerService, AutoCloseable{ long newIdValue = -1; AllocateIdOutputBuilder output = new AllocateIdOutputBuilder(); try { - newIdValue = allocateIdFromLocalPool(localPoolName, idKey); + newIdValue = allocateIdFromLocalPool(poolName, localPoolName, idKey); output.setIdValue(newIdValue); allocateIdRpcBuilder = RpcResultBuilder.success(); allocateIdRpcBuilder.withResult(output.build()); @@ -182,40 +177,69 @@ public class IdManager implements IdManagerService, AutoCloseable{ return Futures.immediateFuture(releaseIdRpcBuilder.build()); } - private long allocateIdFromLocalPool(String localPoolName, String idKey) { + private long allocateIdFromLocalPool(String parentPoolName, String localPoolName, String idKey) { + LOG.trace("Allocating id from local pool {}. Parent pool {}. Idkey {}", localPoolName, parentPoolName, idKey); long newIdValue = -1; - InstanceIdentifier idPoolInstanceIdentifier = IdUtils.getIdPoolInstance(localPoolName); + InstanceIdentifier localIdPoolInstanceIdentifier = IdUtils.getIdPoolInstance(localPoolName); localPoolName = localPoolName.intern(); synchronized (localPoolName) { + InstanceIdentifier parentIdPoolInstanceIdentifier = IdUtils.getIdPoolInstance(parentPoolName); + IdPool parentIdPool = getIdPool(parentIdPoolInstanceIdentifier); + IdPool localPool = null; + try { + localPool = getIdPool(localIdPoolInstanceIdentifier); + } catch (NoSuchElementException e){ + LOG.trace("Creating local pool {} since it was not present", localPoolName); + localPool = IdUtils.createLocalIdPool(localPoolName, parentIdPool); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, localIdPoolInstanceIdentifier, localPool); + IdUtils.lockPool(lockManager, parentPoolName); + try { + updateChildPool(parentPoolName, localPoolName); + } finally { + IdUtils.unlockPool(lockManager, parentPoolName); + } + LOG.debug("Updating global id pool {} with childPool {}", parentPoolName, localPoolName); + } IdEntries newIdEntry; - IdPool pool = getIdPool(idPoolInstanceIdentifier); - List idEntries = pool.getIdEntries(); - - AvailableIdsHolderBuilder availableIds = IdUtils.getAvailableIdsHolderBuilder(pool); - ReleasedIdsHolderBuilder releasedIds = IdUtils.getReleaseIdsHolderBuilder(pool); + List idEntries = parentIdPool.getIdEntries(); + AvailableIdsHolderBuilder availableIds = IdUtils.getAvailableIdsHolderBuilder(localPool); + ReleasedIdsHolderBuilder releasedIds = IdUtils.getReleaseIdsHolderBuilder(localPool); //Calling cleanupExcessIds since there could be excessive ids. - cleanupExcessIds(availableIds, releasedIds, pool.getParentPoolName(), pool.getBlockSize()); + cleanupExcessIds(availableIds, releasedIds, parentPoolName, localPool.getBlockSize()); if (idEntries == null) { idEntries = new LinkedList(); } else { - InstanceIdentifier existingId = IdUtils.getIdEntry(idPoolInstanceIdentifier, idKey); + InstanceIdentifier existingId = IdUtils.getIdEntry(parentIdPoolInstanceIdentifier, idKey); Optional existingIdEntry = MDSALUtil.read(broker, LogicalDatastoreType.CONFIGURATION, existingId); if (existingIdEntry.isPresent()) { newIdValue = existingIdEntry.get().getIdValue(); LOG.debug("Existing id {} for the key {} ", idKey, newIdValue); InstanceIdentifier releasedIdsHolderInstanceIdentifier = InstanceIdentifier - .builder(IdPools.class).child(IdPool.class, new IdPoolKey(localPoolName)).child(ReleasedIdsHolder.class).build(); - MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, releasedIdsHolderInstanceIdentifier, releasedIds.build()); + .builder(IdPools.class).child(IdPool.class, new IdPoolKey(localPoolName)).child(ReleasedIdsHolder.class).build(); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, releasedIdsHolderInstanceIdentifier, releasedIds.build()); return newIdValue; } } - newIdValue = getIdFromPool(pool, availableIds, releasedIds); + newIdValue = getIdFromPool(localPool, availableIds, releasedIds); + LOG.debug("The newIdValue {} for the idKey {}", newIdValue, idKey); newIdEntry = IdUtils.createIdEntries(idKey, newIdValue); idEntries.add(newIdEntry); - pool = new IdPoolBuilder(pool).setIdEntries(idEntries) - .setAvailableIdsHolder(availableIds.build()).setReleasedIdsHolder(releasedIds.build()).build(); - MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, idPoolInstanceIdentifier, pool); - updateChildPool(pool.getParentPoolName(), localPoolName); + LOG.debug("The availablelIds are {}", availableIds.build()); + localPool = new IdPoolBuilder(localPool).setAvailableIdsHolder(availableIds.build()) + .setReleasedIdsHolder(releasedIds.build()).build(); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, localIdPoolInstanceIdentifier, localPool); + updateChildPool(localPool.getParentPoolName(), localPoolName); + //Updating id entries in the parent pool. This will be used for restart scenario + IdUtils.lockPool(lockManager, parentPoolName); + try { + parentIdPool = getIdPool(parentIdPoolInstanceIdentifier); + IdPool parentPool = new IdPoolBuilder(parentIdPool).setIdEntries(idEntries).build(); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, parentIdPoolInstanceIdentifier, parentPool); + } catch (Exception ex) { + LOG.error("Saving of Id entries to parent pool {} failed due to {}", parentPoolName, ex); + } finally { + IdUtils.unlockPool(lockManager, parentPoolName); + } } return newIdValue; } @@ -352,7 +376,7 @@ public class IdManager implements IdManagerService, AutoCloseable{ return 0; } - private long allocateIdBlockFromReleasedIdsHolder(ReleasedIdsHolderBuilder releasedIdsBuilderChild, ReleasedIdsHolderBuilder releasedIdsBuilderParent, IdPool parentIdPool) { + private long allocateIdBlockFromReleasedIdsHolder(ReleasedIdsHolderBuilder releasedIdsBuilderChild, ReleasedIdsHolderBuilder releasedIdsBuilderParent, IdPool parentIdPool) { if (releasedIdsBuilderParent.getAvailableIdCount() == 0) { LOG.debug("Ids unavailable in releasedIds of parent pool {}", parentIdPool); return 0; @@ -403,25 +427,29 @@ public class IdManager implements IdManagerService, AutoCloseable{ } private void releaseIdFromLocalPool(String poolName, String idKey) { - InstanceIdentifier idPoolInstanceIdentifier = IdUtils.getIdPoolInstance(poolName); + InstanceIdentifier localIdPoolInstanceIdentifier = IdUtils.getIdPoolInstance(poolName); poolName = poolName.intern(); synchronized (poolName) { - IdPool pool = getIdPool(idPoolInstanceIdentifier); - List idEntries = pool.getIdEntries(); + IdPool localPool = getIdPool(localIdPoolInstanceIdentifier); + String parentPoolName = localPool.getParentPoolName(); + InstanceIdentifier parentIdPoolInstanceIdentifier = IdUtils.getIdPoolInstance(parentPoolName); + IdPool parentIdPool = getIdPool(parentIdPoolInstanceIdentifier); + List idEntries = parentIdPool.getIdEntries(); List newIdEntries = idEntries; if (idEntries == null) { throw new RuntimeException("Id Entries does not exist"); } - InstanceIdentifier existingId = IdUtils.getIdEntry(idPoolInstanceIdentifier, idKey); + InstanceIdentifier existingId = IdUtils.getIdEntry(parentIdPoolInstanceIdentifier, idKey); Optional existingIdEntryObject = MDSALUtil.read(broker, LogicalDatastoreType.CONFIGURATION, existingId); if (!existingIdEntryObject.isPresent()) { throw new RuntimeException(String.format("Specified Id key %s does not exist in id pool %s", idKey, poolName)); } IdEntries existingIdEntry = existingIdEntryObject.get(); long idValue = existingIdEntry.getIdValue(); - newIdEntries.remove(existingIdEntry); - ReleasedIdsHolderBuilder releasedIds = IdUtils.getReleaseIdsHolderBuilder(pool); - AvailableIdsHolderBuilder availableIds = IdUtils.getAvailableIdsHolderBuilder(pool); + boolean isRemoved = newIdEntries.remove(existingIdEntry); + LOG.debug("The entry {} is removed {}", existingIdEntry, isRemoved); + ReleasedIdsHolderBuilder releasedIds = IdUtils.getReleaseIdsHolderBuilder(localPool); + AvailableIdsHolderBuilder availableIds = IdUtils.getAvailableIdsHolderBuilder(localPool); long delayTime = System.currentTimeMillis() / 1000 + releasedIds.getDelayedTimeSec(); DelayedIdEntries delayedIdEntry = IdUtils.createDelayedIdEntry(idValue, delayTime); List delayedIdEntries = releasedIds.getDelayedIdEntries(); @@ -435,12 +463,23 @@ public class IdManager implements IdManagerService, AutoCloseable{ releasedIds.setDelayedIdEntries(delayedIdEntries); releasedIds.setAvailableIdCount(availableIdCount); //Calling cleanupExcessIds since there could be excessive ids. - cleanupExcessIds(availableIds, releasedIds, pool.getParentPoolName(), pool.getBlockSize()); - pool = new IdPoolBuilder(pool).setIdEntries(newIdEntries) + cleanupExcessIds(availableIds, releasedIds, parentPoolName, localPool.getBlockSize()); + localPool = new IdPoolBuilder(localPool) .setAvailableIdsHolder(availableIds.build()) .setReleasedIdsHolder(releasedIds.build()).build(); - MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, idPoolInstanceIdentifier, pool); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, localIdPoolInstanceIdentifier, localPool); LOG.debug("Released id ({}, {}) from pool {}", idKey, idValue, poolName); + //Updating id entries in the parent pool. This will be used for restart scenario + IdUtils.lockPool(lockManager, parentPoolName); + try { + parentIdPool = getIdPool(parentIdPoolInstanceIdentifier); + IdPool parentPool = new IdPoolBuilder(parentIdPool).setIdEntries(newIdEntries).build(); + MDSALUtil.syncWrite(broker, LogicalDatastoreType.CONFIGURATION, parentIdPoolInstanceIdentifier, parentPool); + } catch (Exception ex) { + LOG.error("Saving of Id entries to parent pool {} failed due to {}", parentPoolName, ex); + } finally { + IdUtils.unlockPool(lockManager, parentPoolName); + } } } @@ -490,8 +529,9 @@ public class IdManager implements IdManagerService, AutoCloseable{ private IdPool getIdPool(InstanceIdentifier idPoolInstanceIdentifier) { Optional idPool = MDSALUtil.read(broker, LogicalDatastoreType.CONFIGURATION, idPoolInstanceIdentifier); if (!idPool.isPresent()) { - throw new RuntimeException(String.format("Specified pool %s does not exist" , idPool)); + throw new NoSuchElementException(String.format("Specified pool %s does not exist" , idPool)); } + LOG.trace("GetIdPool : Read id pool {} ", idPool); return idPool.get(); } diff --git a/idmanager/idmanager-impl/src/test/java/org/opendaylight/idmanager/test/IdManagerTest.java b/idmanager/idmanager-impl/src/test/java/org/opendaylight/idmanager/test/IdManagerTest.java index 9833d04c..1d474478 100644 --- a/idmanager/idmanager-impl/src/test/java/org/opendaylight/idmanager/test/IdManagerTest.java +++ b/idmanager/idmanager-impl/src/test/java/org/opendaylight/idmanager/test/IdManagerTest.java @@ -162,11 +162,12 @@ public class IdManagerTest { public void testAllocateId() throws Exception { AllocateIdInput allocateIdInput = buildAllocateId(globalPoolName, idKey); - Optional expected = Optional.of(globalIdPool); List idEntries = new ArrayList(); idEntries.add(buildIdEntry(idKey2, idValue)); - Optional expectedLocalPool = Optional.of(buildLocalIdPool(blockSize, localPoolName, globalPoolName).setIdEntries(idEntries).build()); - doReturn(Futures.immediateCheckedFuture(expected)).when(mockReadTx).read( + Optional expectedLocalPool = Optional.of(buildLocalIdPool(blockSize, localPoolName, globalPoolName).build()); + IdPool globalIdPool = buildGlobalIdPool(globalPoolName, idStart, idEnd, blockSize, buildChildPool(localPoolName)).setIdEntries(idEntries).build(); + Optional expectedGlobalPool = Optional.of(globalIdPool); + doReturn(Futures.immediateCheckedFuture(expectedGlobalPool)).when(mockReadTx).read( LogicalDatastoreType.CONFIGURATION, identifier); doReturn(Futures.immediateCheckedFuture(expectedLocalPool)).when(mockReadTx).read( LogicalDatastoreType.CONFIGURATION, childIdentifier); @@ -185,7 +186,11 @@ public class IdManagerTest { assertEquals(idStart, pool.getAvailableIdsHolder().getStart().intValue()); assertEquals(idStart + blockSize - 1 , pool.getAvailableIdsHolder().getEnd().intValue()); assertEquals(idStart, pool.getAvailableIdsHolder().getCursor().intValue()); - assertEquals(2, pool.getIdEntries().size()); + } + dataObject = configDataStore.get(identifier); + if (dataObject instanceof IdPool) { + IdPool parentPool = (IdPool) dataObject; + assertEquals(2, parentPool.getIdEntries().size()); } dataObject = configDataStore.get(availableIdsIdentifier); if (dataObject instanceof AvailableIdsHolder) { @@ -201,7 +206,11 @@ public class IdManagerTest { ReleaseIdInput releaseIdInput = createReleaseIdInput(globalPoolName, idKey); List idEntries = new ArrayList(); idEntries.add(buildIdEntry(idKey, idValue)); - Optional expectedLocalPool = Optional.of(buildLocalIdPool(blockSize, localPoolName, globalPoolName).setIdEntries(idEntries).build()); + Optional expectedLocalPool = Optional.of(buildLocalIdPool(blockSize, localPoolName, globalPoolName).build()); + IdPool globalIdPool = buildGlobalIdPool(globalPoolName, idStart, idEnd, blockSize, buildChildPool(localPoolName)).setIdEntries(idEntries).build(); + Optional expectedGlobalPool = Optional.of(globalIdPool); + doReturn(Futures.immediateCheckedFuture(expectedGlobalPool)).when(mockReadTx).read( + LogicalDatastoreType.CONFIGURATION, identifier); doReturn(Futures.immediateCheckedFuture(expectedLocalPool)).when(mockReadTx).read( LogicalDatastoreType.CONFIGURATION, childIdentifier); InstanceIdentifier idEntriesIdentifier = buildIdEntriesIdentifier(idKey); @@ -214,10 +223,14 @@ public class IdManagerTest { DataObject idPoolVal = configDataStore.get(childIdentifier); if (idPoolVal instanceof IdPool) { IdPool pool = (IdPool) idPoolVal; - assertEquals(0, pool.getIdEntries().size()); assertEquals(0, pool.getReleasedIdsHolder().getAvailableIdCount().intValue()); assertEquals(idValue, pool.getReleasedIdsHolder().getDelayedIdEntries().get(0).getId().intValue()); } + idPoolVal = configDataStore.get(identifier); + if (idPoolVal instanceof IdPool) { + IdPool parentPool = (IdPool) idPoolVal; + assertEquals(0, parentPool.getIdEntries().size()); + } } @Test @@ -229,9 +242,13 @@ public class IdManagerTest { idEntries.add(buildIdEntry(idKey2, idValue)); ReleasedIdsHolder excessReleasedIds = createReleasedIdsHolder(0, buildDelayedIdEntries(excessIds), (long) 30); Optional expectedLocalPool = Optional.of(buildLocalIdPool(blockSize, localPoolName, globalPoolName) - .setIdEntries(idEntries).setReleasedIdsHolder(excessReleasedIds) + .setReleasedIdsHolder(excessReleasedIds) .build()); InstanceIdentifier releaseIdsIdentifier = buildReleaseIdsIdentifier(globalPoolName); + IdPool globalIdPool = buildGlobalIdPool(globalPoolName, idStart, idEnd, blockSize, buildChildPool(localPoolName)).setIdEntries(idEntries).build(); + Optional expectedGlobalPool = Optional.of(globalIdPool); + doReturn(Futures.immediateCheckedFuture(expectedGlobalPool)).when(mockReadTx).read( + LogicalDatastoreType.CONFIGURATION, identifier); doReturn(Futures.immediateCheckedFuture(expected)).when(mockReadTx) .read(LogicalDatastoreType.CONFIGURATION, releaseIdsIdentifier); doReturn(Futures.immediateCheckedFuture(expectedLocalPool)).when( @@ -250,7 +267,11 @@ public class IdManagerTest { IdPool pool = (IdPool) dataObject; assertEquals(localPoolName, pool.getPoolName()); assertEquals(excessIds.length - 3, pool.getReleasedIdsHolder().getAvailableIdCount().intValue()); - assertEquals(2, pool.getIdEntries().size()); + } + dataObject = configDataStore.get(identifier); + if (dataObject instanceof IdPool) { + IdPool parentPool = (IdPool) dataObject; + assertEquals(2, parentPool.getIdEntries().size()); } dataObject = configDataStore.get(releaseIdsIdentifier); if (dataObject instanceof ReleasedIdsHolder) { @@ -285,9 +306,13 @@ public class IdManagerTest { IdPool pool = (IdPool) dataObject; assertEquals(localPoolName, pool.getPoolName()); assertEquals(1, pool.getReleasedIdsHolder().getDelayedIdEntries().size()); - assertEquals(1, pool.getIdEntries().size()); assertEquals(1, pool.getReleasedIdsHolder().getAvailableIdCount().intValue()); } + dataObject = configDataStore.get(identifier); + if (dataObject instanceof IdPool) { + IdPool parentPool = (IdPool) dataObject; + assertEquals(1, parentPool.getIdEntries().size()); + } dataObject = configDataStore.get(releaseIdsIdentifier); if (dataObject instanceof ReleasedIdsHolder) { ReleasedIdsHolder releasedIds = (ReleasedIdsHolder) dataObject; @@ -345,7 +370,7 @@ public class IdManagerTest { } private InstanceIdentifier buildIdEntriesIdentifier(String idKey) { - InstanceIdentifier.InstanceIdentifierBuilder idEntriesBuilder = childIdentifier + InstanceIdentifier.InstanceIdentifierBuilder idEntriesBuilder = identifier .builder().child(IdEntries.class, new IdEntriesKey(idKey)); InstanceIdentifier idEntry = idEntriesBuilder.build(); return idEntry; -- 2.36.6