diff --git a/ambry-store/src/main/java/com/github/ambry/store/StorageManager.java b/ambry-store/src/main/java/com/github/ambry/store/StorageManager.java index ff4b3c327b..ccfbb9feb9 100644 --- a/ambry-store/src/main/java/com/github/ambry/store/StorageManager.java +++ b/ambry-store/src/main/java/com/github/ambry/store/StorageManager.java @@ -517,7 +517,11 @@ DiskManager addDisk(DiskId diskId) { }); } - + /** + * Add a new store to the storage manager for file copy based replication post filecopy is completed. + * @param replica the {@link ReplicaId} of the {@link Store} which would be added. + * @return + */ @Override public boolean addBlobStoreForFileCopy(ReplicaId replica) { if (partitionToDiskManager.containsKey(replica.getPartitionId())) { @@ -552,15 +556,18 @@ public boolean addBlobStore(ReplicaId replica) { return true; } + /** + * Build inmemory state for file copy based replication post filecopy is completed. + * @param replica the {@link ReplicaId} of the {@link Store} for which store needs to be built + */ @Override public void buildStateForFileCopy(ReplicaId replica){ - PartitionId partitionId = replica.getPartitionId(); - if (replica == null) { - logger.error("No existing replica found for partition {} in partitionNameToReplicaId", partitionId.getId()); - throw new StateTransitionException( - "Existing replica " + partitionId.getId() + " is not found in clustermap for " + currentNode, ReplicaNotFound); + logger.error("ReplicaId is null"); + throw new StateTransitionException("ReplicaId null is not found in clustermap for " + currentNode, ReplicaNotFound); } + PartitionId partitionId = replica.getPartitionId(); + if (!addBlobStoreForFileCopy(replica)){ // We have decreased the available disk space in HelixClusterManager#getDiskForBootstrapReplica. Increase it // back since addition of store failed. diff --git a/ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java b/ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java index ab95636f00..ea03a769b9 100644 --- a/ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java +++ b/ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java @@ -270,6 +270,14 @@ public void scheduleAndControlCompactionTest() throws Exception { shutdownAndAssertStoresInaccessible(storageManager, replicas); } + /** + * Helper util to add blobs to a given Store + * @param store store to add blob to. + * @param size size in bytes of the randomized blob + * @param expiresAtMs expiry in milliseconds to be set for the blob + * @return {@link MockId} the mock id of the blob added + * @throws StoreException + */ public MockId addRandomBlobToStore(Store store, long size, long expiresAtMs) throws StoreException { final Random random = new Random(); short accountId = Utils.getRandomShort(TestUtils.RANDOM); @@ -288,6 +296,7 @@ public MockId addRandomBlobToStore(Store store, long size, long expiresAtMs) thr /** * Test buildStateForFileCopy with newly created {@link ReplicaId}. + * @throws Exception */ @Test public void buildStateForFileCopyTest() throws Exception { @@ -326,6 +335,66 @@ public void buildStateForFileCopyTest() throws Exception { } + /** + * Test buildStateForFileCopy with newly created {@link ReplicaId} for failure to add an already started blob store. + * @throws Exception + */ + @Test + public void buildStateForFileCopyFailureToAddBlobStoreTest() throws Exception { + generateConfigs(true, false); + MockDataNodeId localNode = clusterMap.getDataNodes().get(0); + int newMountPathIndex = 3; + // add new MountPath to local node + File f = File.createTempFile("ambry", ".tmp"); + File mountFile = + new File(f.getParent(), "mountpathfile" + MockClusterMap.PLAIN_TEXT_PORT_START_NUMBER + newMountPathIndex); + MockClusterMap.deleteFileOrDirectory(mountFile); + assertTrue("Couldn't create mount path directory", mountFile.mkdir()); + localNode.addMountPaths(Collections.singletonList(mountFile.getAbsolutePath())); + int newPartitionId = 803; + PartitionId newPartition = + new MockPartitionId(newPartitionId, MockClusterMap.DEFAULT_PARTITION_CLASS, clusterMap.getDataNodes(), newMountPathIndex); + StorageManager storageManager = createStorageManager(localNode, metricRegistry, null); + storageManager.start(); + // test add store onto a new disk, which should succeed + assertTrue("Add new store should succeed", storageManager.addBlobStore(newPartition.getReplicaIds().get(0))); + assertNotNull("The store shouldn't be null because new store is successfully added", + storageManager.getStore(newPartition, false)); + // This should fail since the newPartition already has store started. + assertFalse("Add store which is already existing should fail", storageManager.addBlobStoreForFileCopy(newPartition.getReplicaIds().get(0))); + } + + /** + * Test buildStateForFileCopy with {@link ReplicaId} as null. + * @throws Exception + */ + @Test(expected = StateTransitionException.class) + public void buildStateForFileCopyReplicaNullFailureTest() throws Exception { + generateConfigs(true, false); + MockDataNodeId localNode = clusterMap.getDataNodes().get(0); + int newMountPathIndex = 3; + // add new MountPath to local node + File f = File.createTempFile("ambry", ".tmp"); + File mountFile = + new File(f.getParent(), "mountpathfile" + MockClusterMap.PLAIN_TEXT_PORT_START_NUMBER + newMountPathIndex); + MockClusterMap.deleteFileOrDirectory(mountFile); + assertTrue("Couldn't create mount path directory", mountFile.mkdir()); + localNode.addMountPaths(Collections.singletonList(mountFile.getAbsolutePath())); + int newPartitionId = 803; + PartitionId newPartition = + new MockPartitionId(newPartitionId, MockClusterMap.DEFAULT_PARTITION_CLASS, clusterMap.getDataNodes(), newMountPathIndex); + StorageManager storageManager = createStorageManager(localNode, metricRegistry, null); + storageManager.start(); + // test add store onto a new disk, which should succeed + assertTrue("Add new store should succeed", storageManager.addBlobStore(newPartition.getReplicaIds().get(0))); + assertNotNull("The store shouldn't be null because new store is successfully added", + storageManager.getStore(newPartition, false)); + // This should fail since the newPartition already has store started. + assertFalse("Add store which is already existing should fail", storageManager.addBlobStoreForFileCopy(newPartition.getReplicaIds().get(0))); + storageManager.buildStateForFileCopy(null); + } + + /** * Test add new BlobStore with given {@link ReplicaId}. */