State Build implementation for File Copy based replication #2954

piyujai · 2024-11-27T06:35:33Z

This PR handles initializing BlobStore with Log, Index, BlobStoreCompactor, RemoteTokenTracker, etc post filecopy is completed in the filecopy based replication.

Bootstrap State Build Design Doc

Key Changes:

buildStateForFileCopy: New method to build in-memory state when a file copy completes under file copy based replication. It throws an exception if ReplicaId is null and adjusts disk space on failure.
addBlobStoreForFileCopy: Added to DiskManager and StorageManager for adding BlobStores during file copy replication. It includes initializing of BlobStore with Log, Index, BlobStoreCompactor and RemoteTokenTracker, etc. It includes error handling for failed store creation.
Error Handling: Enhanced logging and handling for failed store additions and bootstrap replica issues.

Test Coverage:

Added tests for various file copy scenarios, including successful and failed store additions and handling null ReplicaId, successful state build post blob writes to a new store, etc.
Testing with sealed log files with setting of active segments and rollover of files is working correctly in local testing.

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

DevenAhluwalia · 2024-12-17T07:09:30Z

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java

+        // create a bootstrap-in-progress file to distinguish it from regular stores (the file will be checked during
+        // BOOTSTRAP -> STANDBY transition)
+        createBootstrapFileIfAbsent(replica);
+        logger.info("New store is successfully added into DiskManager.");


nit: We should log partitionId in this log.

DevenAhluwalia · 2024-12-17T07:10:50Z

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java

+        succeed = true;
+      }
+    } catch (Exception e) {
+      logger.error("Failed to start new added store {} or add requirements to disk allocator", replica.getPartitionId(),


Log statement needs to be tweaked as we are not making a call to diskSpaceAllocator now.

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

DevenAhluwalia · 2024-12-17T07:23:49Z

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

+    Store store = dm.getStore(newPartition, false);
+    MockId id1 = addRandomBlobToStore(store, 100, Utils.Infinite_Time);
+    MockId id2 = addRandomBlobToStore(store, 200, Utils.Infinite_Time);
+    // Create storage manager again to create disk managers again


For my understanding, why is shutting down the storageManager and re-initiating it, relevant / necessary?

We need to shut it down to ensure that BlobStores are shut down and state build is able to start the BlobStores again. This is to test the flow where existing blobs are written to disk and then building state is attempted by adding a store via stateBuild flow.

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

DevenAhluwalia · 2024-12-17T07:39:05Z

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

+   * @throws Exception
+   */
+  @Test
+  public void buildStateForFileCopyFailureToAddBlobStoreTest() throws Exception {


Rename: AddingDuplicateBlobStoreShouldFailTest*

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

aga9900 · 2024-12-17T07:46:41Z

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

+   */
+  @Override
+  public boolean addBlobStoreForFileCopy(ReplicaId replica) {
+    if (partitionToDiskManager.containsKey(replica.getPartitionId())) {


This will get added as part of pre-file store, so it might occur that the this always returns true. We need to check for the existence of store here explicitely.

We should instead check below map for existence of store in disk manager.
private final ConcurrentHashMap<PartitionId, BlobStore> stores = new ConcurrentHashMap<>();

Yes, diskmanager will be started already in pre-filecopy steps. Instead, I have put the check of partitionToDiskManager not having the partitionId key and fetching the diskManager from this map itself. Checking of store's existence is handled in the addBlobStoreForFileCopy call.

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

aga9900 · 2024-12-20T16:36:57Z

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

+      logger.error("Failed to add new store into DiskManager");
+      return false;
+    }
+    partitionToDiskManager.put(replica.getPartitionId(), diskManager);


This should already be present when we add the Blob Store after file copy.
Steps happening before this:

Addition of Replica object.

File Copy Trigger -> This will add FileStore in StorageManager which goes through DiskManager. This step also should create an entry in partitionToDiskManager.

…ication

aga9900 · 2024-12-21T05:40:34Z

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java

+        compactionManager.addBlobStore(store);
+        // add new created store into in-memory data structures.
+        stores.put(replica.getPartitionId(), store);
+        partitionToReplicaMap.put(replica.getPartitionId(), replica);


As the name suggest, we should be adding an entry to this map much sooner than after the file copy finishes.

Done, we should handle it in prefilecopy. Please make a note.

aga9900 · 2024-12-21T05:42:28Z

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java

+      logger.error("Failed to add new store into DiskManager");
+      return false;
+    }
+    partitionNameToReplicaId.put(replica.getPartitionId().toPathString(), replica);


Same as this: https://github.com/linkedin/ambry/pull/2954/files#r1894565174

Done, we should handle it in prefilecopy. Please make a note.

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java

aga9900

Added couple of comments.

changes are done.

DevenAhluwalia

🚢

piyujai force-pushed the bootstrap_state_build_changes branch 2 times, most recently from 80d1bce to 1124d46 Compare November 28, 2024 11:08

piyujai force-pushed the bootstrap_state_build branch from 0948730 to ef290ec Compare November 29, 2024 06:39

piyujai force-pushed the bootstrap_state_build_changes branch from 6e10bcf to 944356e Compare November 29, 2024 06:43

piyujai force-pushed the bootstrap_state_build branch from ef290ec to 350dd47 Compare November 29, 2024 06:48

piyujai marked this pull request as draft November 29, 2024 06:49

piyujai force-pushed the bootstrap_state_build branch 2 times, most recently from b608e52 to 2e147ea Compare December 16, 2024 13:24

piyujai marked this pull request as ready for review December 16, 2024 18:26

piyujai changed the title ~~WIP: Changes for state build~~ State Build implementation for File Copy based replication Dec 16, 2024

piyujai force-pushed the bootstrap_state_build branch 3 times, most recently from 4f7c463 to 664acbe Compare December 16, 2024 21:33

piyujai changed the base branch from bootstrap_state_build_changes to master December 17, 2024 06:09

piyujai changed the base branch from master to bootstrap-optimisation-lld-flow December 17, 2024 06:09

aga9900 reviewed Dec 17, 2024

View reviewed changes

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java Show resolved Hide resolved

DevenAhluwalia previously requested changes Dec 17, 2024

View reviewed changes

aga9900 reviewed Dec 17, 2024

View reviewed changes

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java Outdated Show resolved Hide resolved

aga9900 reviewed Dec 17, 2024

View reviewed changes

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java Show resolved Hide resolved

justinlin-linkedin reviewed Dec 18, 2024

View reviewed changes

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java Show resolved Hide resolved

ambry-store/src/main/java/com/github/ambry/store/DiskManager.java Show resolved Hide resolved

ambry-store/src/main/java/com/github/ambry/store/StorageManager.java Show resolved Hide resolved

piyujai force-pushed the bootstrap_state_build branch from 664acbe to 76073d6 Compare December 20, 2024 09:56

piyujai changed the base branch from bootstrap-optimisation-lld-flow to master December 20, 2024 09:56

aga9900 reviewed Dec 20, 2024

View reviewed changes

Jai Balani added 6 commits December 21, 2024 04:29

Added state build changes for StoreManager interface

8ff616f

State build unsupported operation handling for CloudStorageManager

c716f17

StoreManager methods handling for MockStorageManager

3536a14

Added handling for addition of new blob store for filecopy based repl…

d09fe35

…ication

State build implementation changes for StorageManager

12ff26f

Added tests for state build for filecopy based replication

c767164

Failure tests added and minor refactoring

2685135

piyujai force-pushed the bootstrap_state_build branch 3 times, most recently from 82b5da6 to 1b97cc7 Compare December 20, 2024 23:14

aga9900 reviewed Dec 21, 2024

View reviewed changes

ambry-store/src/test/java/com/github/ambry/store/StorageManagerTest.java Show resolved Hide resolved

aga9900 previously requested changes Dec 21, 2024

View reviewed changes

piyujai force-pushed the bootstrap_state_build branch 2 times, most recently from 6d69c20 to 4098a89 Compare December 23, 2024 07:36

PR comments changes

f32df47

piyujai force-pushed the bootstrap_state_build branch from 4098a89 to f32df47 Compare December 23, 2024 07:40

aga9900 self-requested a review December 23, 2024 07:52

DevenAhluwalia approved these changes Dec 23, 2024

View reviewed changes

piyujai merged commit 81a7b6c into master Dec 23, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

State Build implementation for File Copy based replication #2954

State Build implementation for File Copy based replication #2954

piyujai commented Nov 27, 2024 •

edited

Loading

DevenAhluwalia Dec 17, 2024

DevenAhluwalia Dec 17, 2024

DevenAhluwalia Dec 17, 2024

piyujai Dec 20, 2024

DevenAhluwalia Dec 17, 2024

aga9900 Dec 17, 2024 •

edited

Loading

aga9900 Dec 17, 2024

piyujai Dec 20, 2024

aga9900 Dec 20, 2024 •

edited

Loading

aga9900 Dec 21, 2024 •

edited

Loading

piyujai Dec 23, 2024

aga9900 Dec 21, 2024

piyujai Dec 23, 2024

aga9900 left a comment

DevenAhluwalia left a comment

State Build implementation for File Copy based replication #2954

State Build implementation for File Copy based replication #2954

Conversation

piyujai commented Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aga9900 Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aga9900 Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

aga9900 Dec 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aga9900 left a comment

Choose a reason for hiding this comment

DevenAhluwalia left a comment

Choose a reason for hiding this comment

piyujai commented Nov 27, 2024 •

edited

Loading

aga9900 Dec 17, 2024 •

edited

Loading

aga9900 Dec 20, 2024 •

edited

Loading

aga9900 Dec 21, 2024 •

edited

Loading