Bug 7531 : Different ids allocated for same key 52/50352/9
authorKency Kurian <kency.kurian@ericsson.com>
Thu, 12 Jan 2017 11:19:05 +0000 (16:49 +0530)
committerKency Kurian <kency.kurian@ericsson.com>
Wed, 25 Jan 2017 04:28:47 +0000 (09:58 +0530)
Fixed the issue when allocate id for same idKey returns different idValue.
 - Updates a map with a future object if the map is empty during allocate.
 - Also remove from map once data is written in order to avoid stale
   entries.
 - If there is already an entry wait until the future completes.

Also while testing found out that the deleteIdPool RPC was not working
since the listener registration got missed after blueprint migration.

Fixed the JUnit failures.

Change-Id: I0ad4882603ef6472dcd3b780e31175a1a19b49d1
Signed-off-by: Kency Kurian <kency.kurian@ericsson.com>
idmanager/idmanager-impl/src/main/java/org/opendaylight/genius/idmanager/IdManager.java
idmanager/idmanager-impl/src/main/java/org/opendaylight/genius/idmanager/IdPoolListener.java
idmanager/idmanager-impl/src/main/java/org/opendaylight/genius/idmanager/IdUtils.java
idmanager/idmanager-impl/src/main/java/org/opendaylight/genius/idmanager/jobs/UpdateIdEntryJob.java
idmanager/idmanager-impl/src/test/java/org/opendaylight/genius/idmanager/test/IdManagerTest.java

index 7ffcd85cd37f8d58cdc94779329c6a50c4b50375..98a5da306a52519624f3ccd250460c9365d9b8b6 100644 (file)
@@ -12,6 +12,7 @@ import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastor
 
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.ListenableFuture;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -20,16 +21,21 @@ import java.util.List;
 import java.util.Map;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
@@ -142,7 +148,7 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
         ReleasedIdHolder releasedIdHolder = new ReleasedIdHolder(idUtils, releasedIdsHolder.getDelayedTimeSec());
         releasedIdHolder.setAvailableIdCount(releasedIdsHolder.getAvailableIdCount());
         List<DelayedIdEntries> delayedEntries = releasedIdsHolder.getDelayedIdEntries();
-        List<DelayedIdEntry> delayedIdEntryInCache = new ArrayList<>();
+        List<DelayedIdEntry> delayedIdEntryInCache = new CopyOnWriteArrayList<>();
         if (delayedEntries != null) {
             delayedIdEntryInCache = delayedEntries
                     .parallelStream()
@@ -216,6 +222,9 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
             output.setIdValue(newIdValue);
             futureResult = RpcResultBuilder.<AllocateIdOutput>success().withResult(output.build()).buildFuture();
         } catch (OperationFailedException | IdManagerException e) {
+            java.util.Optional.ofNullable(
+                    idUtils.allocatedIdMap.remove(idUtils.getUniqueKey(poolName, idKey)))
+                    .ifPresent(futureId -> futureId.completeExceptionally(e));
             futureResult = buildFailedRpcResultFuture("allocateId failed: " + input.toString(), e);
         }
         return futureResult;
@@ -239,6 +248,9 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
             output.setIdValues(newIdValuesList);
             futureResult = RpcResultBuilder.<AllocateIdRangeOutput>success().withResult(output.build()).buildFuture();
         } catch (OperationFailedException | IdManagerException e) {
+            java.util.Optional.ofNullable(
+                    idUtils.allocatedIdMap.remove(idUtils.getUniqueKey(poolName, idKey)))
+                    .ifPresent(futureId -> futureId.completeExceptionally(e));
             futureResult = buildFailedRpcResultFuture("allocateIdRange failed: " + input.toString(), e);
         }
         return futureResult;
@@ -296,14 +308,28 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
         return failedRpcResultBuilder.buildFuture();
     }
 
-    private List<Long> allocateIdFromLocalPool(String parentPoolName, String localPoolName, String idKey, long size)
-            throws OperationFailedException, IdManagerException {
+    private List<Long> allocateIdFromLocalPool(String parentPoolName, String localPoolName,
+            String idKey, long size) throws OperationFailedException, IdManagerException {
         if (LOG.isDebugEnabled()) {
             LOG.debug("Allocating id from local pool {}. Parent pool {}. Idkey {}", localPoolName, parentPoolName,
                     idKey);
         }
-        long newIdValue = -1;
         List<Long> newIdValuesList = new ArrayList<>();
+        String uniqueIdKey = idUtils.getUniqueKey(parentPoolName, idKey);
+        CompletableFuture<List<Long>> futureIdValues = new CompletableFuture<>();
+        CompletableFuture<List<Long>> existingFutureIdValue =
+                idUtils.allocatedIdMap.putIfAbsent(uniqueIdKey, futureIdValues);
+        if (existingFutureIdValue != null) {
+            try {
+                newIdValuesList = existingFutureIdValue.get();
+                return newIdValuesList;
+            } catch (InterruptedException | ExecutionException e) {
+                LOG.warn("Could not obtain id from existing futureIdValue for idKey {} and pool {}.",
+                        idKey, parentPoolName);
+                throw new IdManagerException(e.getMessage(), e);
+            }
+        }
+        long newIdValue = -1;
         localPoolName = localPoolName.intern();
         InstanceIdentifier<IdPool> parentIdPoolInstanceIdentifier = idUtils.getIdPoolInstance(parentPoolName);
         InstanceIdentifier<IdEntries> existingId = idUtils.getIdEntry(parentIdPoolInstanceIdentifier, idKey);
@@ -313,6 +339,13 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Existing ids {} for the key {} ", newIdValuesList, idKey);
             }
+            // Inform other waiting threads about this new value.
+            futureIdValues.complete(newIdValuesList);
+            // This is to avoid stale entries in the map. If this thread had populated the map,
+            // then the entry should be removed.
+            if (existingFutureIdValue == null) {
+                idUtils.allocatedIdMap.remove(uniqueIdKey);
+            }
             return newIdValuesList;
         }
         //This get will not help in concurrent reads. Hence the same read needs to be done again.
@@ -369,10 +402,11 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
         if (LOG.isDebugEnabled()) {
             LOG.debug("The newIdValues {} for the idKey {}", newIdValuesList, idKey);
         }
-        idUtils.releaseIdLatchMap.put(parentPoolName + idKey, new CountDownLatch(1));
+        idUtils.releaseIdLatchMap.put(uniqueIdKey, new CountDownLatch(1));
         UpdateIdEntryJob job = new UpdateIdEntryJob(parentPoolName, localPoolName, idKey, newIdValuesList, broker,
                 idUtils);
         DataStoreJobCoordinator.getInstance().enqueueJob(parentPoolName, job, IdUtils.RETRY_COUNT);
+        futureIdValues.complete(newIdValuesList);
         return newIdValuesList;
     }
 
@@ -380,22 +414,26 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
             throws OperationFailedException, IdManagerException {
         while (true) {
             IdHolder releasedIds = localIdPool.getReleasedIds();
-            Optional<Long> releasedId = releasedIds.allocateId();
+            Optional<Long> releasedId = Optional.absent();
+            releasedId = releasedIds.allocateId();
             if (releasedId.isPresent()) {
-                IdHolderSyncJob poolSyncJob = new IdHolderSyncJob(localIdPool.getPoolName(), releasedIds, broker,
-                        idUtils);
-                DataStoreJobCoordinator.getInstance().enqueueJob(localIdPool.getPoolName(), poolSyncJob,
-                        IdUtils.RETRY_COUNT);
+                IdHolderSyncJob poolSyncJob =
+                        new IdHolderSyncJob(localIdPool.getPoolName(), localIdPool.getReleasedIds(), broker,
+                                idUtils);
+                DataStoreJobCoordinator.getInstance().enqueueJob(localIdPool.getPoolName(),
+                        poolSyncJob, IdUtils.RETRY_COUNT);
                 return releasedId.get();
             }
+            Optional<Long> availableId = Optional.absent();
             IdHolder availableIds = localIdPool.getAvailableIds();
             if (availableIds != null) {
-                Optional<Long> availableId = availableIds.allocateId();
+                availableId = availableIds.allocateId();
                 if (availableId.isPresent()) {
-                    IdHolderSyncJob poolSyncJob = new IdHolderSyncJob(localIdPool.getPoolName(), availableIds, broker,
-                            idUtils);
-                    DataStoreJobCoordinator.getInstance().enqueueJob(localIdPool.getPoolName(), poolSyncJob,
-                            IdUtils.RETRY_COUNT);
+                    IdHolderSyncJob poolSyncJob =
+                            new IdHolderSyncJob(localIdPool.getPoolName(), localIdPool.getAvailableIds(),
+                                    broker, idUtils);
+                    DataStoreJobCoordinator.getInstance().enqueueJob(localIdPool.getPoolName(),
+                            poolSyncJob, IdUtils.RETRY_COUNT);
                     return availableId.get();
                 }
             }
@@ -578,7 +616,7 @@ public class IdManager implements IdManagerService, IdManagerMonitor {
 
     private void releaseIdFromLocalPool(String parentPoolName, String localPoolName, String idKey)
             throws ReadFailedException, IdManagerException {
-        String idLatchKey = parentPoolName + idKey;
+        String idLatchKey = idUtils.getUniqueKey(parentPoolName, idKey);
         java.util.Optional.ofNullable(idUtils.releaseIdLatchMap.get(idLatchKey)).ifPresent(latch -> {
             try {
                 latch.await(5, TimeUnit.SECONDS);
index eab5d3f8ab222ab62f0994d876bb9fd0b742333b..ce7585abe74ce6c41f7cf16a271a0880b983681a 100644 (file)
@@ -7,9 +7,12 @@
  */
 package org.opendaylight.genius.idmanager;
 
+import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.datastoreutils.AsyncClusteredDataTreeChangeListenerBase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.IdPools;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.id.pools.IdPool;
@@ -35,6 +38,12 @@ public class IdPoolListener extends AsyncClusteredDataTreeChangeListenerBase<IdP
         this.idUtils = idUtils;
     }
 
+    @PostConstruct
+    public void start() throws Exception {
+        registerListener(LogicalDatastoreType.CONFIGURATION, this.broker);
+        LOG.info("IdPoolListener listener Started");
+    }
+
     @Override
     protected void remove(InstanceIdentifier<IdPool> identifier, IdPool del) {
         String parentPoolName = del.getParentPoolName();
index 3cd427cc4535f9dcadd42a97b4325e554d7776ab..7cd17b4c899ff896987841835affa871f11de6b8 100644 (file)
@@ -10,16 +10,20 @@ package org.opendaylight.genius.idmanager;
 
 import com.google.common.base.Optional;
 import com.google.common.net.InetAddresses;
+
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+
 import javax.inject.Singleton;
+
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.idmanager.ReleasedIdHolder.DelayedIdEntry;
@@ -59,6 +63,7 @@ public class IdUtils {
     private static final int DEFAULT_BLOCK_SIZE_DIFF = 10;
     public static final int RETRY_COUNT = 6;
 
+    public final ConcurrentHashMap<String, CompletableFuture<List<Long>>> allocatedIdMap = new ConcurrentHashMap<>();
     public final ConcurrentHashMap<String, CountDownLatch> releaseIdLatchMap = new ConcurrentHashMap<>();
     private final ConcurrentHashMap<String, Integer> poolUpdatedMap = new ConcurrentHashMap<>();
 
@@ -68,7 +73,7 @@ public class IdUtils {
         bladeId = InetAddresses.coerceToInteger(InetAddress.getLocalHost());
     }
 
-    protected InstanceIdentifier<IdEntries> getIdEntry(InstanceIdentifier<IdPool> poolName, String idKey) {
+    public InstanceIdentifier<IdEntries> getIdEntry(InstanceIdentifier<IdPool> poolName, String idKey) {
         InstanceIdentifier.InstanceIdentifierBuilder<IdEntries> idEntriesBuilder = poolName
                 .builder().child(IdEntries.class, new IdEntriesKey(idKey));
         InstanceIdentifier<IdEntries> idEntry = idEntriesBuilder.build();
@@ -315,4 +320,8 @@ public class IdUtils {
     public void removeFromPoolUpdatedMap(String localPoolName) {
         poolUpdatedMap.remove(localPoolName);
     }
+
+    public String getUniqueKey(String parentPoolName, String idKey) {
+        return parentPoolName + idKey;
+    }
 }
index e8d9367e898ded73aa412d63165df257aa26b636..8bafc1bab790e4ffd36d2042c5d974279a7238ce 100644 (file)
@@ -50,8 +50,11 @@ public class UpdateIdEntryJob implements Callable<List<ListenableFuture<Void>>>
             tx.delete(CONFIGURATION, idUtils.getIdEntriesInstanceIdentifier(parentPoolName, idKey));
         }
         tx.submit().checkedGet();
-        Optional.ofNullable(idUtils.releaseIdLatchMap.get(parentPoolName + idKey))
+        String uniqueIdKey = idUtils.getUniqueKey(parentPoolName, idKey);
+        Optional.ofNullable(idUtils.releaseIdLatchMap.get(uniqueIdKey))
             .ifPresent(latch -> latch.countDown());
+        // Once the id is written to DS, removing the id value from map.
+        idUtils.allocatedIdMap.remove(uniqueIdKey);
         return Collections.emptyList();
     }
 }
index 39b6b79c1a81b36f4a755d1c43dc525e9b3aab79..80ad809f554cb68d39a819dadd164b18edd58ad1 100644 (file)
@@ -18,19 +18,20 @@ import static org.mockito.Mockito.when;
 
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.Futures;
+
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.Before;
 import org.junit.Ignore;
@@ -85,7 +86,7 @@ import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
 @RunWith(MockitoJUnitRunner.class)
 public class IdManagerTest {
 
-    Map<InstanceIdentifier<?>, DataObject> configDataStore = new HashMap<>();
+    ConcurrentHashMap<InstanceIdentifier<?>, DataObject> configDataStore = new ConcurrentHashMap<>();
     @Mock DataBroker dataBroker;
     @Mock ReadOnlyTransaction mockReadTx;
     @Mock WriteTransaction mockWriteTx;
@@ -128,20 +129,19 @@ public class IdManagerTest {
             configDataStore.put(invocation.getArgumentAt(1, KeyedInstanceIdentifier.class),
                     invocation.getArgumentAt(2, IdPool.class));
             return null;
-        }).when(mockWriteTx).put(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), any(IdPool.class), eq(true));
+        }).when(mockWriteTx).put(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), Matchers.any(), eq(true));
         doAnswer(invocation -> {
             configDataStore.put(invocation.getArgumentAt(1, KeyedInstanceIdentifier.class),
                     invocation.getArgumentAt(2, IdPool.class));
             return null;
-        }).when(mockWriteTx).merge(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), any(ChildPools.class),
-                eq(true));
+        }).when(mockWriteTx).merge(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), Matchers.any(), eq(true));
         doAnswer(invocation -> {
             configDataStore.put(invocation.getArgumentAt(1, KeyedInstanceIdentifier.class),
                     invocation.getArgumentAt(2, IdPool.class));
             return null;
-        }).when(mockWriteTx).merge(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), any(IdPool.class), eq(true));
+        }).when(mockWriteTx).merge(eq(LogicalDatastoreType.CONFIGURATION), Matchers.any(), Matchers.any());
         doAnswer(invocation -> {
-            configDataStore.put(invocation.getArgumentAt(1, KeyedInstanceIdentifier.class), null);
+            configDataStore.remove(invocation.getArgumentAt(1, KeyedInstanceIdentifier.class));
             return null;
         }).when(mockWriteTx).delete(eq(LogicalDatastoreType.CONFIGURATION), Matchers.<InstanceIdentifier<IdPool>>any());
 
@@ -165,7 +165,7 @@ public class IdManagerTest {
         IdPool pool;
         assertTrue(result.get().isSuccessful());
         // Just to ensure the local pool is also written. Even if it is not triggered Test case will pass.
-        Thread.sleep(100);
+        waitUntilJobIsDone();
         assertTrue(configDataStore.size() > 0);
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         if (dataObject instanceof IdPool) {
@@ -213,7 +213,7 @@ public class IdManagerTest {
         AllocateIdInput allocateIdInput = buildAllocateId(poolName, idKey);
         Future<RpcResult<AllocateIdOutput>> result = idManager.allocateId(allocateIdInput);
         assertTrue(result.get().isSuccessful());
-        Thread.sleep(100);
+        waitUntilJobIsDone();
         assertTrue(configDataStore.size() > 0);
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         if (dataObject instanceof IdPool) {
@@ -260,7 +260,8 @@ public class IdManagerTest {
         ReleaseIdInput releaseIdInput = createReleaseIdInput(poolName, idKey);
         Future<RpcResult<Void>> result = idManager.releaseId(releaseIdInput);
         assertTrue(result.get().isSuccessful());
-        Thread.sleep(100);
+        waitUntilJobIsDone();
+
         assertTrue(configDataStore.size() > 0);
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         if (dataObject instanceof IdPool) {
@@ -372,7 +373,8 @@ public class IdManagerTest {
         AllocateIdInput allocateIdInput = buildAllocateId(poolName, idKey);
         Future<RpcResult<AllocateIdOutput>> result = idManager.allocateId(allocateIdInput);
         assertTrue(result.get().isSuccessful());
-        Thread.sleep(3);
+        waitUntilJobIsDone();
+
         assertTrue(configDataStore.size() > 0);
         InstanceIdentifier<IdPool> localPoolIdentifier = buildInstanceIdentifier(localPoolName);
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
@@ -408,15 +410,18 @@ public class IdManagerTest {
         List<IdPool> listOfIdPool = new ArrayList<>();
         listOfIdPool.add(localPool);
         listOfIdPool.add(globalIdPool);
+        // Pre-loading the map so that we can remove it when it is removed from DS.
+        configDataStore.put(parentPoolIdentifier, globalIdPool);
+        configDataStore.put(localPoolIdentifier, localPool);
         setupMocks(listOfIdPool);
         Optional<IdPool> expected = Optional.of(globalIdPool);
         doReturn(Futures.immediateCheckedFuture(expected)).when(mockReadTx).read(
                 LogicalDatastoreType.CONFIGURATION, parentPoolIdentifier);
         DeleteIdPoolInput deleteIdPoolInput = createDeleteIdPoolInput(poolName);
         Future<RpcResult<Void>> result = idManager.deleteIdPool(deleteIdPoolInput);
-        Thread.sleep(3);
+        waitUntilJobIsDone();
         assertTrue(result.get().isSuccessful());
-        assertTrue(configDataStore.size() > 0);
+        assertTrue(configDataStore.size() == 0);
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         assertEquals(dataObject, null);
         dataObject = configDataStore.get(parentPoolIdentifier);
@@ -425,24 +430,13 @@ public class IdManagerTest {
 
     @Test
     public void testMultithreadedIdAllocationFromAvailableIds() throws Exception {
-        List<IdPool> listOfIdPool = new ArrayList<>();
-        IdPool localIdPool = buildLocalIdPool(blockSize, idStart, idStart + blockSize - 1, idStart - 1, localPoolName,
-                poolName).build();
-        listOfIdPool.add(localIdPool);
-        int poolSize = 10;
-        IdPool globalIdPool = buildGlobalIdPool(poolName, idStart, poolSize, blockSize,
-                buildChildPool(localPoolName)).build();
-        listOfIdPool.add(globalIdPool);
-        setupMocks(listOfIdPool);
-        doReturn(Futures.immediateCheckedFuture(Optional.of(globalIdPool))).when(mockReadTx).read(
-                LogicalDatastoreType.CONFIGURATION, parentPoolIdentifier);
-        ExecutorService executor = Executors.newCachedThreadPool();
+        setupMockForMultiThreads(false);
         int numberOfTasks = 3;
         CountDownLatch latch = new CountDownLatch(numberOfTasks);
         Set<Long> idSet = new CopyOnWriteArraySet<>();
-        requestIdsConcurrently(latch, numberOfTasks, idSet);
+        requestIdsConcurrently(latch, numberOfTasks, idSet, false);
         latch.await();
-        Thread.sleep(500);
+        waitUntilJobIsDone();
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         if (dataObject instanceof IdPool) {
             IdPool pool = (IdPool) dataObject;
@@ -450,28 +444,15 @@ public class IdManagerTest {
         }
     }
 
-    @Ignore
+    @Test
     public void testMultithreadedIdAllocationFromReleasedIds() throws Exception {
-        List<DelayedIdEntries> delayedIdEntries = buildDelayedIdEntries(new long[] {100, 101});
-        ReleasedIdsHolder expectedReleasedIds = createReleasedIdsHolder(2, delayedIdEntries , 0);
-        List<IdPool> listOfIdPool = new ArrayList<>();
-        IdPool localIdPool = buildLocalIdPool(blockSize, idStart, idStart + blockSize - 1,
-                idStart + blockSize - 1, localPoolName, poolName).build();
-        listOfIdPool.add(localIdPool);
-        int poolSize = 10;
-        IdPool globalIdPool = buildGlobalIdPool(poolName, idStart, poolSize, blockSize,
-                buildChildPool(localPoolName)).setReleasedIdsHolder(expectedReleasedIds).build();
-        listOfIdPool.add(globalIdPool);
-        setupMocks(listOfIdPool);
-        doReturn(Futures.immediateCheckedFuture(Optional.of(globalIdPool))).when(mockReadTx).read(
-                LogicalDatastoreType.CONFIGURATION, parentPoolIdentifier);
-        ExecutorService executor = Executors.newCachedThreadPool();
+        setupMockForMultiThreads(true);
         int numberOfTasks = 3;
         CountDownLatch latch = new CountDownLatch(numberOfTasks);
         Set<Long> idSet = new CopyOnWriteArraySet<>();
-        requestIdsConcurrently(latch, numberOfTasks, idSet);
+        requestIdsConcurrently(latch, numberOfTasks, idSet, false);
         latch.await();
-        Thread.sleep(2000);
+        waitUntilJobIsDone();
         DataObject dataObject = configDataStore.get(localPoolIdentifier);
         if (dataObject instanceof IdPool) {
             IdPool pool = (IdPool) dataObject;
@@ -479,6 +460,76 @@ public class IdManagerTest {
         }
     }
 
+    @Test
+    public void testMultithreadedIdAllocationForSameKeyFromAvailableIds() throws Exception {
+        setupMockForMultiThreads(false);
+        int numberOfTasks = 3;
+        CountDownLatch latch = new CountDownLatch(numberOfTasks);
+        Set<Long> idSet = new CopyOnWriteArraySet<>();
+        requestIdsConcurrently(latch, numberOfTasks, idSet, true);
+        latch.await();
+        assertTrue(idSet.size() == 1);
+        waitUntilJobIsDone();
+        DataObject dataObject = configDataStore.get(localPoolIdentifier);
+        if (dataObject instanceof IdPool) {
+            IdPool pool = (IdPool) dataObject;
+            assertTrue(idStart == pool.getAvailableIdsHolder().getCursor());
+        }
+    }
+
+    @Test
+    public void testMultithreadedIdAllocationForSameKeyFromReleasedIds() throws Exception {
+        setupMockForMultiThreads(true);
+        int numberOfTasks = 3;
+        CountDownLatch latch = new CountDownLatch(numberOfTasks);
+        Set<Long> idSet = new CopyOnWriteArraySet<>();
+        requestIdsConcurrently(latch, numberOfTasks, idSet, true);
+        latch.await();
+        assertTrue(idSet.size() == 1);
+        waitUntilJobIsDone();
+        DataObject dataObject = configDataStore.get(localPoolIdentifier);
+        if (dataObject instanceof IdPool) {
+            IdPool pool = (IdPool) dataObject;
+            assertTrue(pool.getReleasedIdsHolder().getAvailableIdCount() == 2);
+        }
+    }
+
+    private void setupMockForMultiThreads(boolean isRelease) throws ReadFailedException, UnknownHostException {
+        List<IdPool> listOfIdPool = new ArrayList<>();
+        IdPoolBuilder localIdPool =
+                buildLocalIdPool(blockSize, idStart, idStart + blockSize, idStart - 1,
+                        localPoolName, poolName);
+        int poolSize = 10;
+        IdPool globalIdPool = buildGlobalIdPool(poolName, idStart, poolSize, blockSize,
+                buildChildPool(localPoolName)).build();
+        listOfIdPool.add(globalIdPool);
+        setupMocks(listOfIdPool);
+        doReturn(Futures.immediateCheckedFuture(Optional.of(globalIdPool))).when(mockReadTx).read(
+                LogicalDatastoreType.CONFIGURATION, parentPoolIdentifier);
+
+        List<DelayedIdEntries> delayedIdEntries = buildDelayedIdEntries(new long[] {100, 101, 102});
+        ReleasedIdsHolder expectedReleasedIds = createReleasedIdsHolder(3, delayedIdEntries , 0);
+        if (isRelease) {
+            localIdPool.setReleasedIdsHolder(expectedReleasedIds);
+        }
+        listOfIdPool.add(localIdPool.build());
+        listOfIdPool.add(globalIdPool);
+        setupMocks(listOfIdPool);
+        doAnswer(invocation -> {
+            DataObject result = configDataStore.get(idUtils.getIdEntry(parentPoolIdentifier, idKey));
+            if (result == null) {
+                return Futures.immediateCheckedFuture(Optional.absent());
+            }
+            if (result instanceof IdEntries) {
+                return Futures.immediateCheckedFuture(Optional.of((IdEntries) result));
+            }
+            return Futures.immediateCheckedFuture(Optional.absent());
+        }).when(mockReadTx).read(LogicalDatastoreType.CONFIGURATION, idUtils.getIdEntry(parentPoolIdentifier, idKey));
+
+        doReturn(Futures.immediateCheckedFuture(Optional.of(globalIdPool))).when(mockReadTx).read(
+                LogicalDatastoreType.CONFIGURATION, parentPoolIdentifier);
+    }
+
     private InstanceIdentifier<ReleasedIdsHolder> buildReleaseIdsIdentifier(
             String poolName) {
         InstanceIdentifier<ReleasedIdsHolder> releasedIds = InstanceIdentifier
@@ -576,7 +627,7 @@ public class IdManagerTest {
         List<DelayedIdEntries> delayedIdEntriesList = new ArrayList<>();
         for (long idValue : idValues) {
             DelayedIdEntries delayedIdEntries = new DelayedIdEntriesBuilder().setId(idValue)
-                    .setReadyTimeSec(System.currentTimeMillis() / 1000).build();
+                    .setReadyTimeSec(0L).build();
             delayedIdEntriesList.add(delayedIdEntries);
         }
         return delayedIdEntriesList;
@@ -590,20 +641,29 @@ public class IdManagerTest {
         return childPoolsList;
     }
 
-    private void requestIdsConcurrently(CountDownLatch latch, int numberOfTasks, Set<Long> idSet) {
+    private void requestIdsConcurrently(CountDownLatch latch, int numberOfTasks, Set<Long> idSet, boolean isSameKey) {
         ExecutorService executor = Executors.newCachedThreadPool();
         for (int i = 0; i < numberOfTasks; i++) {
             executor.execute(new Runnable() {
                 @Override
                 public void run() {
-                    Future<RpcResult<AllocateIdOutput>> result = idManager.allocateId(buildAllocateId(poolName,
-                            Thread.currentThread().getName()));
-                    Long idValue = null;
+                    Future<RpcResult<AllocateIdOutput>> result;
+                    if (!isSameKey) {
+                        result = idManager.allocateId(buildAllocateId(poolName,
+                                Thread.currentThread().getName()));
+                    } else {
+                        result = idManager.allocateId(buildAllocateId(poolName,
+                                idKey));
+                    }
                     try {
                         if (result.get().isSuccessful()) {
-                            idValue = result.get().getResult().getIdValue();
-                            assertTrue(idValue <= idStart + blockSize - 1);
-                            assertTrue(idSet.add(idValue));
+                            Long idValue = result.get().getResult().getIdValue();
+                            assertTrue(idValue <= idStart + blockSize);
+                            if (isSameKey) {
+                                idSet.add(idValue);
+                            } else {
+                                assertTrue(idSet.add(idValue));
+                            }
                         } else {
                             RpcError error = result.get().getErrors().iterator().next();
                             assertTrue(error.getCause().getMessage().contains("Ids exhausted for pool : " + poolName));
@@ -617,4 +677,8 @@ public class IdManagerTest {
             });
         }
     }
+
+    private void waitUntilJobIsDone() throws InterruptedException {
+        TimeUnit.SECONDS.sleep(1);
+    }
 }