devmadhuu commented on code in PR #10702:
URL: https://github.com/apache/ozone/pull/10702#discussion_r3568156464


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/StaleRecoveringContainerScrubbingService.java:
##########


Review Comment:
   I think, we also need to re-validate if currentTime < deadline in 
`RecoveringContainerScrubbingTask.call()` to avoid any race condition as 
mentioned earlier.
   
   ```suggestion
         Long deadline = 
containerSet.getRecoveringContainerMap().get(containerID);                      
                                                                                
                         
     if (deadline == null || containerSet.getCurrentTime() < deadline) {        
                                                                                
                                              
       return new BackgroundTaskResult.EmptyTaskResult();   // updated after we 
were queued — skip                                                              
                                             
     }                                                                          
                                                                                
                                              
     con.markContainerUnhealthy();   
   ```



##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/ECBlockOutputStream.java:
##########
@@ -83,11 +85,46 @@ public ECBlockOutputStream(
       Token<? extends TokenIdentifier> token,
       ContainerClientMetrics clientMetrics, StreamBufferArgs streamBufferArgs,
       Supplier<ExecutorService> executorServiceSupplier
+  ) throws IOException {
+    this(blockID, xceiverClientManager, pipeline, bufferPool, config, token, 
clientMetrics,
+        streamBufferArgs, executorServiceSupplier, false);
+  }
+
+  @SuppressWarnings("checkstyle:ParameterNumber")
+  public ECBlockOutputStream(
+      BlockID blockID,
+      XceiverClientFactory xceiverClientManager,
+      Pipeline pipeline,
+      BufferPool bufferPool,
+      OzoneClientConfig config,
+      Token<? extends TokenIdentifier> token,
+      ContainerClientMetrics clientMetrics, StreamBufferArgs streamBufferArgs,
+      Supplier<ExecutorService> executorServiceSupplier,
+      boolean denyContainerAutoCreate
   ) throws IOException {
     super(blockID, -1, xceiverClientManager,
         pipeline, bufferPool, config, token, clientMetrics, streamBufferArgs, 
executorServiceSupplier);
     // In EC stream, there will be only one node in pipeline.
     this.datanodeDetails = pipeline.getClosestNode();
+    this.denyContainerAutoCreate = denyContainerAutoCreate;
+    if (denyContainerAutoCreate) {
+      getContainerBlockData().addMetadata(containerCreatableFalseKv());

Review Comment:
   this field is added to getContainerBlockData() and to each `ChunkInfo` via 
`decorateChunkInfo`, but `BlockData/ChunkInfo `are persisted. This will 
increase our block and chunk metadata on disk. Can we avoid this proto change 
to be persisted ? Can we use this `denyContainerAutoCreate` to pass through 
`ContainerCommandRequestProto`  as an option field, 
so`ECReconstructionCoordinator's `stream sets it on the 
`WriteChunk/PutBlock/PutSmallFile` requests (instead of decorateChunkInfo + 
`getContainerBlockData().addMetadata`



-- 
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