peterxcli commented on code in PR #8915:
URL: https://github.com/apache/ozone/pull/8915#discussion_r2319437109


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/SendContainerRequestHandler.java:
##########
@@ -84,7 +86,15 @@ public void onNext(SendContainerRequest req) {
 
       if (containerId == -1) {
         containerId = req.getContainerID();
-        volume = importer.chooseNextVolume();
+        
+        // Use replicate size if available, otherwise fall back to default
+        if (firstRequest && req.hasReplicateSize()) {

Review Comment:
   I believe if the condition  (containerId == -1) is true, then it must be the 
first request.
   
   ```suggestion
           if (req.hasReplicateSize()) {
   ```



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECMisReplicationHandler.java:
##########
@@ -83,6 +83,7 @@ protected int sendReplicateCommands(
           // For EC containers, we need to track the replica index which is
           // to be replicated, so add it to the command.
           cmd.setReplicaIndex(replica.getReplicaIndex());
+          cmd.setReplicateSize(containerInfo.getUsedBytes());

Review Comment:
   Is this no-op change? (question only. I guess this is for the consistence of 
pb message, am I correct?)



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/SendContainerRequestHandler.java:
##########
@@ -94,6 +104,8 @@ public void onNext(SendContainerRequest req) {
 
         LOG.info("Accepting container {}", req.getContainerID());
       }
+      
+      firstRequest = false;

Review Comment:
   ```suggestion
   ```



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -142,11 +142,11 @@ private static void deleteFileQuietely(Path tarFilePath) {
     }
   }
 
-  HddsVolume chooseNextVolume() throws IOException {
+  HddsVolume chooseNextVolume(long spaceReserved) throws IOException {
     // Choose volume that can hold both container in tmp and dest directory
     return volumeChoosingPolicy.chooseVolume(
         StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()),
-        getDefaultReplicationSpace());
+        spaceReserved);

Review Comment:
   ```suggestion
           spaceToReserve);
   ```
   



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -142,11 +142,11 @@ private static void deleteFileQuietely(Path tarFilePath) {
     }
   }
 
-  HddsVolume chooseNextVolume() throws IOException {
+  HddsVolume chooseNextVolume(long spaceReserved) throws IOException {

Review Comment:
   ```suggestion
     HddsVolume chooseNextVolume(long spaceToReserve) throws IOException {
   ```
   



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/DownloadAndImportReplicator.java:
##########
@@ -71,9 +71,20 @@ public void replicate(ReplicationTask task) {
     LOG.info("Starting replication of container {} from {} using {}",
         containerID, sourceDatanodes, compression);
     HddsVolume targetVolume = null;
+    
+    // Use replicate size from command if available, otherwise use default
+    long spaceReserved = 0;
+    Long replicateSize = task.getReplicateSize();
+    if (replicateSize != null) {

Review Comment:
   for the fallback, would it be possible that `replicateSize` is 0?



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -142,11 +142,11 @@ private static void deleteFileQuietely(Path tarFilePath) {
     }
   }
 
-  HddsVolume chooseNextVolume() throws IOException {
+  HddsVolume chooseNextVolume(long spaceReserved) throws IOException {
     // Choose volume that can hold both container in tmp and dest directory
     return volumeChoosingPolicy.chooseVolume(
         StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()),
-        getDefaultReplicationSpace());
+        spaceReserved);

Review Comment:
   ditto all other occupations.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/SendContainerRequestHandler.java:
##########
@@ -54,6 +54,8 @@ class SendContainerRequestHandler
   private Path path;
   private CopyContainerCompression compression;
   private final ZeroCopyMessageMarshaller<SendContainerRequest> marshaller;
+  private long spaceReserved = 0;
+  private boolean firstRequest = true;

Review Comment:
   ```suggestion
     private long spaceToReserve = 0;
   ```



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -142,11 +142,11 @@ private static void deleteFileQuietely(Path tarFilePath) {
     }
   }
 
-  HddsVolume chooseNextVolume() throws IOException {
+  HddsVolume chooseNextVolume(long spaceReserved) throws IOException {

Review Comment:
   ditto all other occupations.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to