Failure tests added and minor refactoring

linkedin · Dec 16, 2024 · 664acbe · 664acbe
1 parent 2e147ea
commit 664acbe
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 6 deletions.
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
@@ -514,7 +514,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())) {
@@ -549,15 +553,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
@@ -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}.
    */