Re: [PR] HDDS-11253. Handle corrupted merkle tree files [ozone]

["errose28 (via GitHub)" <gi...@apache.org>]

errose28 commented on code in PR #7083:
URL: https://github.com/apache/ozone/pull/7083#discussion_r1747519083


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -133,42 +167,47 @@ private Lock getWriteLock(long containerID) {
     return fileLock.get(containerID).writeLock();
   }
 
-  private ContainerProtos.ContainerChecksumInfo read(ContainerData data) throws IOException {
+  private Optional<ContainerProtos.ContainerChecksumInfo.Builder> read(ContainerData data) throws IOException {
     long containerID = data.getContainerID();
+    File checksumFile = getContainerChecksumFile(data);
     Lock readLock = getReadLock(containerID);
     readLock.lock();
     try {
-      File checksumFile = getContainerChecksumFile(data);
-      // If the checksum file has not been created yet, return an empty instance.
-      // Since all writes happen as part of an atomic read-modify-write cycle that requires a write lock, two empty
-      // instances for the same container obtained only under the read lock will not conflict.
       if (!checksumFile.exists()) {
-        LOG.debug("No checksum file currently exists for container {} at the path {}. Returning an empty instance.",
-            containerID, checksumFile);
-        return ContainerProtos.ContainerChecksumInfo.newBuilder()
-            .setContainerID(containerID)
-            .build();
+        LOG.debug("No checksum file currently exists for container {} at the path {}", containerID, checksumFile);
+        return Optional.empty();
       }
       try (FileInputStream inStream = new FileInputStream(checksumFile)) {
         return captureLatencyNs(metrics.getReadContainerMerkleTreeLatencyNS(),
-            () -> ContainerProtos.ContainerChecksumInfo.parseFrom(inStream));
+            () -> Optional.of(ContainerProtos.ContainerChecksumInfo.parseFrom(inStream).toBuilder()));
       }
     } catch (IOException ex) {
       metrics.incrementMerkleTreeReadFailures();
       throw new IOException("Error occurred when reading container merkle tree for containerID "
-              + data.getContainerID(), ex);
+              + data.getContainerID() + " at path " + checksumFile, ex);
     } finally {
       readLock.unlock();
     }
   }
 
   private void write(ContainerData data, ContainerProtos.ContainerChecksumInfo checksumInfo) throws IOException {
+    // Make sure callers filled in required fields before writing.
+    Preconditions.assertTrue(checksumInfo.hasContainerID());
+
+    File checksumFile = getContainerChecksumFile(data);
+    File tmpChecksumFile = getTmpContainerChecksumFile(data);
+
     Lock writeLock = getWriteLock(data.getContainerID());
     writeLock.lock();
-    try (FileOutputStream outStream = new FileOutputStream(getContainerChecksumFile(data))) {
-      captureLatencyNs(metrics.getWriteContainerMerkleTreeLatencyNS(),
-          () -> checksumInfo.writeTo(outStream));
+    try (FileOutputStream tmpOutputStream = new FileOutputStream(tmpChecksumFile)) {
+      // Write to a tmp file and rename it into place.
+      captureLatencyNs(metrics.getWriteContainerMerkleTreeLatencyNS(), () -> {
+        checksumInfo.writeTo(tmpOutputStream);
+        Files.move(tmpChecksumFile.toPath(), checksumFile.toPath(), REPLACE_EXISTING);

Review Comment:
   After offline discussion, we are going to assume rewrite with overwrite is working on any posix system Ozone is deployed on.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org