errose28 commented on code in PR #9947:
URL: https://github.com/apache/ozone/pull/9947#discussion_r3164377089
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksDB.java:
##########
@@ -75,6 +75,14 @@ public static ManagedRocksDB openReadOnly(
);
}
+ public static ManagedRocksDB openAsSecondary(
Review Comment:
@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.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
@@ -311,17 +311,20 @@ public VolumeCheckResult checkDbHealth(File dbFile)
throws InterruptedException
}
try (ManagedOptions managedOptions = new ManagedOptions();
- ManagedRocksDB ignored = ManagedRocksDB.openReadOnly(managedOptions,
dbFile.toString())) {
+ ManagedRocksDB ignored =
+ ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(),
getTmpDir().getPath())) {
Review Comment:
In [this
comment](https://github.com/apache/ozone/pull/9947#issuecomment-4346456680) it
looks like the logs are being written to the disk check dir, but it seems like
the code doesn't match.
```suggestion
ManagedRocksDB.openAsSecondary(managedOptions,
dbFile.toString(), getDiskCheckDir().getPath())) {
```
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
Review Comment:
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.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
@@ -311,17 +311,20 @@ public VolumeCheckResult checkDbHealth(File dbFile)
throws InterruptedException
}
try (ManagedOptions managedOptions = new ManagedOptions();
- ManagedRocksDB ignored = ManagedRocksDB.openReadOnly(managedOptions,
dbFile.toString())) {
+ ManagedRocksDB ignored =
+ ManagedRocksDB.openAsSecondary(managedOptions, dbFile.toString(),
getTmpDir().getPath())) {
Review Comment:
> 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?
--
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]
