Skip to content

HDDS-14859. Use RocksDb secondary instance for validating volumes. #9947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ptlrs wants to merge 9 commits into
base: master
Choose a base branch
from
+13 −2
Draft

HDDS-14859. Use RocksDb secondary instance for validating volumes. #9947

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9f50fba
HDDS-14859. Ignore transient errors while validating RocksDb on volumes.
ptlrs Mar 18, 2026
c2ffea9
HDDS-14859. Add support for opening RocksDB as a secondary for volume…
ptlrs Mar 24, 2026
7190e4e
Merge remote-tracking branch 'refs/remotes/upstream/master' into HDDS…
ptlrs Apr 18, 2026
a2160c2
HDDS-14859. Add configurable retry gap for disk checks and validate i…
ptlrs Apr 18, 2026
7dc2bfd
HDDS-14859. Add configurable retry attempts for RocksDb disk health c…
ptlrs Apr 18, 2026
02a6986
HDDS-14859. Remove disk check retry logic and related configurations.
ptlrs Apr 23, 2026
dafaf60
HDDS-14859. Add setter for diskCheckTimeout in DatanodeConfiguration.
ptlrs Apr 23, 2026
13bba73
Merge remote-tracking branch 'refs/remotes/upstream/master' into HDDS…
ptlrs Apr 27, 2026
1986d60
HDDS-14859. Remove deprecated RocksDb disk IO health check configurat…
ptlrs Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions ...ain/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,14 @@ public class DatanodeConfiguration extends ReconfigurableConfig {
)
private boolean isDiskCheckEnabled = true;

@Config(key = "hdds.datanode.rocksdb.disk.check.io.test.enabled",
Copy link
Copy Markdown
Contributor

@ChenSammi ChenSammi Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we rename it to "hdds.datanode.disk.check.rocksdb.io.test.enabled", so that all the disk check property will share the "hdds.datanode.disk.check" prefix?

Copy link
Copy Markdown
Contributor Author

@ptlrs ptlrs Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved the switch to its own PR in #10149.

I have updated it there.

defaultValue = "true",
type = ConfigType.BOOLEAN,
tags = {DATANODE},
description = "The configuration to enable or disable RocksDb disk IO checks."
)
private boolean isRocksDbDiskCheckEnabled = true;

@Config(key = "hdds.datanode.disk.check.io.failures.tolerated",
defaultValue = "1",
type = ConfigType.INT,
Expand Down Expand Up @@ -936,6 +944,10 @@ public boolean isDiskCheckEnabled() {
return isDiskCheckEnabled;
}

public boolean isRocksDbDiskCheckEnabled() {
return isRocksDbDiskCheckEnabled;
}

public Duration getDiskCheckSlidingWindowTimeout() {
return diskCheckSlidingWindowTimeout;
}
Expand Down
9 changes: 6 additions & 3 deletions ...ner-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,22 +306,25 @@ public synchronized VolumeCheckResult check(@Nullable Boolean unused)

@VisibleForTesting
public VolumeCheckResult checkDbHealth(File dbFile) throws InterruptedException {
Copy link
Copy Markdown
Contributor

@errose28 errose28 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add javadoc to this method to clarify that our goal is to check global files like CURRENT and MANIFEST, and that verifying the contents of all SST files/values in the DB is done by the container data scanner.

if (!getDiskCheckEnabled()) {
if (!(getDiskCheckEnabled() && getDatanodeConfig().isRocksDbDiskCheckEnabled())) {
return VolumeCheckResult.HEALTHY;
}

try (ManagedOptions managedOptions = new ManagedOptions();
ManagedRocksDB ignored = ManagedRocksDB.openReadOnly(managedOptions, dbFile.toString())) {
ManagedRocksDB ignored =
ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(), getTmpDir().getPath())) {
Comment thread
ptlrs marked this conversation as resolved.
Copy link
Copy Markdown
Contributor

@errose28 errose28 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this comment it looks like the logs are being written to the disk check dir, but it seems like the code doesn't match.

Suggested change
ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(), getTmpDir().getPath())) {
ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(), getDiskCheckDir().getPath())) {

Copy link
Copy Markdown
Contributor

@errose28 errose28 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the PR to remove these log files every time after closing the secondary instance

I'm also not seeing this. Are there still some commits pending?

// 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);

LOG.error("Could not open Volume DB located at {}", dbFile, e);
getIoTestSlidingWindow().add();
}


if (getIoTestSlidingWindow().isExceeded()) {
LOG.error("Failed to open the database at \"{}\" for HDDS volume {}: " +
"encountered more than the {} tolerated failures.",
Expand Down
8 changes: 8 additions & 0 deletions ...managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksDB.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ public static ManagedRocksDB openReadOnly(
);
}

public static ManagedRocksDB openAsSecondary(
Copy link
Copy Markdown
Contributor

@errose28 errose28 Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptlrs thanks for all the research you've done on secondary instances. It would be great to put a summary as a javadoc above this method about how they work and what we can expect vs. readonly instances.

final ManagedOptions options,
final String dbPath,
final String secondaryDbLogFilePath)
throws RocksDBException {
return new ManagedRocksDB(RocksDB.openAsSecondary(options, dbPath, secondaryDbLogFilePath));
Comment thread
ptlrs marked this conversation as resolved.
}

public static ManagedRocksDB open(
final DBOptions options, final String path,
final List<ColumnFamilyDescriptor> columnFamilyDescriptors,
Expand Down