hemantk-12 commented on code in PR #7943:
URL: https://github.com/apache/ozone/pull/7943#discussion_r1972470945


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -98,6 +98,81 @@ public long putBlock(Container container, BlockData data,
         data, endOfBlock);
   }
 
+  @Override
+  public long putBlockForClosedContainer(Container container, BlockData data, 
boolean overwriteBcsId)
+          throws IOException {
+    Preconditions.checkNotNull(data, "BlockData cannot be null for put " +
+        "operation.");
+    Preconditions.checkState(data.getContainerID() >= 0, "Container Id " +
+        "cannot be negative");

Review Comment:
   nit:
   ```suggestion
   Preconditions.checkNotNull(data, "BlockData cannot be null for put 
operation.");
       Preconditions.checkState(data.getContainerID() >= 0, "Container Id 
cannot be negative");
   ```



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java:
##########
@@ -174,8 +175,25 @@ public void writeChunk(Container container, BlockID 
blockID, ChunkInfo info,
       ChunkUtils.validateChunkSize(channel, info, chunkFile.getName());
     }
 
-    ChunkUtils
-        .writeData(channel, chunkFile.getName(), data, offset, len, volume);
+    long fileLengthBeforeWrite;
+    try {
+      fileLengthBeforeWrite = channel.size();
+    } catch (IOException e) {
+      throw new StorageContainerException("IO error encountered while " +
+          "getting the file size for " + chunkFile.getName(), 
CHUNK_FILE_INCONSISTENCY);

Review Comment:
   nit:
   ```suggestion
         throw new StorageContainerException("Encountered an error while 
getting the file size for " + chunkFile.getName(),
             CHUNK_FILE_INCONSISTENCY);
   ```



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -98,6 +98,81 @@ public long putBlock(Container container, BlockData data,
         data, endOfBlock);
   }
 
+  @Override
+  public long putBlockForClosedContainer(Container container, BlockData data, 
boolean overwriteBcsId)

Review Comment:
   nit: there are lots of warnings due to raw use `Container`. Maybe we change 
`BlockManager` to type T something like this `BlockManager<T extends 
ContainerData>`. It could be a separate PR if we agree.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java:
##########
@@ -98,6 +98,81 @@ public long putBlock(Container container, BlockData data,
         data, endOfBlock);
   }
 
+  @Override
+  public long putBlockForClosedContainer(Container container, BlockData data, 
boolean overwriteBcsId)
+          throws IOException {
+    Preconditions.checkNotNull(data, "BlockData cannot be null for put " +
+        "operation.");
+    Preconditions.checkState(data.getContainerID() >= 0, "Container Id " +
+        "cannot be negative");
+
+    KeyValueContainerData containerData = (KeyValueContainerData) 
container.getContainerData();
+
+    // We are not locking the key manager since RocksDB serializes all actions
+    // against a single DB. We rely on DB level locking to avoid conflicts.
+    try (DBHandle db = BlockUtils.getDB(containerData, config)) {
+      // This is a post condition that acts as a hint to the user.
+      // Should never fail.
+      Preconditions.checkNotNull(db, DB_NULL_ERR_MSG);
+
+      long bcsId = data.getBlockCommitSequenceId();
+      long containerBCSId = containerData.getBlockCommitSequenceId();
+
+      // Check if the block is already present in the DB of the container to 
determine whether
+      // the blockCount is already incremented for this block in the DB or not.
+      long localID = data.getLocalID();
+      boolean incrBlockCount = false;
+
+      // update the blockData as well as BlockCommitSequenceId here
+      try (BatchOperation batch = db.getStore().getBatchHandler()
+          .initBatchOperation()) {
+        // If block already exists in the DB, blockCount should not be 
incremented.
+        if 
(db.getStore().getBlockDataTable().get(containerData.getBlockKey(localID)) == 
null) {
+          // Block does not exist in DB => blockCount needs to be
+          // incremented when the block is added into DB.

Review Comment:
   nit: This is a redundant comment. Similar comment is on line#129.



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