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


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
@@ -335,6 +378,20 @@ public VolumeCheckResult checkDbHealth(File dbFile) throws 
InterruptedException
     return VolumeCheckResult.HEALTHY;
   }
 
+  private void deleteSecondaryInstanceLogs(File secondaryDir) {

Review Comment:
   Instead of this method we should create a subdirectory under `diskCheckDir` 
just for the logs, and then `rm -rf` that subdirectory without regard for its 
contents. We can `mkdir -p` this directory on every disk check.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
@@ -304,24 +305,66 @@ public synchronized VolumeCheckResult check(@Nullable 
Boolean unused)
     return checkDbHealth(dbFile);
   }
 
+  /**
+   * Verifies the per-volume RocksDB's global state files (CURRENT, MANIFEST,
+   * OPTIONS) by opening the DB in secondary mode. A successful open implies
+   * those files are readable and internally consistent and that the
+   * referenced SST file names match what RocksDB expects.
+   *
+   * <p>This check intentionally does <b>not</b> read or checksum SST file
+   * contents or any individual key/value. Per-block / per-key integrity is
+   * verified by the container data scanner, which scans containers (and
+   * their RocksDB rows) on its own schedule.
+   *
+   * <p>The volume is only marked {@link VolumeCheckResult#FAILED} once the
+   * configured threshold of failures is exceeded, matching the parent class's
+   * intermittent-error tolerance. Open failures whose underlying RocksDB
+   * status is {@code IOError(NoSpace)} are not counted: {@code 
openAsSecondary}
+   * writes its info LOG into the disk-check directory, so an out-of-space
+   * failure there is unrelated to DB integrity. Any other status — permission
+   * denied, missing path, corruption, generic IO error — is still counted as
+   * a real failure.
+   */
   @VisibleForTesting
   public VolumeCheckResult checkDbHealth(File dbFile) throws 
InterruptedException {
-    if (!(getDiskCheckEnabled() && 
getDatanodeConfig().isRocksDbDiskCheckEnabled())) {
+    if (!getDiskCheckEnabled()) {
       return VolumeCheckResult.HEALTHY;
     }
 
+    File secondaryDir = getDiskCheckDir();
     try (ManagedOptions managedOptions = new ManagedOptions();
-         ManagedRocksDB ignored = ManagedRocksDB.openReadOnly(managedOptions, 
dbFile.toString())) {
+         ManagedRocksDB ignored =
+             ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(), 
secondaryDir.getPath())) {
       // Do nothing. Only check if rocksdb is accessible.
       LOG.debug("Successfully opened the database at \"{}\" for HDDS volume 
{}.", dbFile, getStorageDir());
     } catch (Exception e) {
       if (Thread.currentThread().isInterrupted()) {
         throw new InterruptedException("Check of database for volume " + this 
+ " interrupted.");
       }
-      LOG.warn("Could not open Volume DB located at {}", dbFile, e);
-      getIoTestSlidingWindow().add();
+
+      // openAsSecondary writes its info LOG into secondaryDir. If that write
+      // fails because the disk is full, RocksDB surfaces the failure as
+      // IOError(NoSpace) (mapped from ENOSPC). That is unrelated to DB
+      // integrity, so don't count it against the sliding window. Any other
+      // status (permission denied, missing path, corruption, generic IO
+      // error) is still treated as a real failure.
+      if (ManagedRocksDB.isNoSpaceFailure(e)) {
+        LOG.warn("Skipping RocksDB health-check failure accounting for volume 
{}: " +
+                "secondary open returned IOError(NoSpace) for {}. Cause: {}",
+            this, secondaryDir, e.toString());

Review Comment:
   We should pass the exception object directly to the log message and let 
log4j format it.



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