jojochuang commented on code in PR #8173:
URL: https://github.com/apache/ozone/pull/8173#discussion_r2023752444
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestDBConfigFromFile.java:
##########
@@ -90,23 +70,21 @@ public void readFromFile() throws IOException {
@Test
public void readFromFileInvalidConfig() throws IOException {
- final List<String> families =
- Arrays.asList(StringUtils.bytes2String(RocksDB.DEFAULT_COLUMN_FAMILY),
- "First", "Second", "Third",
- "Fourth", "Fifth",
- "Sixth");
- final List<ColumnFamilyDescriptor> columnFamilyDescriptors =
- new ArrayList<>();
- for (String family : families) {
- columnFamilyDescriptors.add(
- new ColumnFamilyDescriptor(family.getBytes(StandardCharsets.UTF_8),
- new ColumnFamilyOptions()));
- }
-
- final DBOptions options = DBConfigFromFile.readFromFile("badfile.db.ini",
- columnFamilyDescriptors);
-
+ final DBOptions options =
DBConfigFromFile.readDBOptionsFromFile(Paths.get("badfile.db.ini"));
// This has to return a Null, since we have config defined for badfile.db
assertNull(options);
}
+
+ @Test
Review Comment:
Please also check that edge cases (such as an empty or misconfigured ini
file) are covered in the test suite.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java:
##########
@@ -110,17 +113,14 @@ public DatanodeSchemaThreeDBDefinition(String dbPath,
DatanodeDBProfile dbProfile = DatanodeDBProfile
.getProfile(config.getEnum(HDDS_DB_PROFILE, HDDS_DEFAULT_DB_PROFILE));
- ManagedColumnFamilyOptions cfOptions =
- dbProfile.getColumnFamilyOptions(config);
- // Use prefix seek to mitigating seek overhead.
- // See: https://github.com/facebook/rocksdb/wiki/Prefix-Seek
Review Comment:
Please add these comments in getCFOptions()
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBProfile.java:
##########
@@ -44,51 +49,98 @@ public String toString() {
}
@Override
- public ManagedColumnFamilyOptions getColumnFamilyOptions() {
- // Write Buffer Size -- set to 128 MB
- final long writeBufferSize = toLong(StorageUnit.MB.toBytes(128));
+ public ManagedDBOptions getDBOptions(Path dbPath) {
+ ManagedDBOptions option = null;
+ try {
+ if (dbPath != null) {
+ option = DBConfigFromFile.readDBOptionsFromFile(dbPath);
+ }
+ } catch (IOException ex) {
+ LOG.error("Unable to read RocksDB DBOptions from {}, exception {}",
dbPath, ex);
+ }
+ if (option != null) {
+ LOG.info("Using RocksDB DBOptions from {}.ini file", dbPath);
+ } else {
+ final int maxBackgroundCompactions = 4;
+ final int maxBackgroundFlushes = 2;
+ final long bytesPerSync = toLong(StorageUnit.MB.toBytes(1.00));
+ final boolean createIfMissing = true;
+ final boolean createMissingColumnFamilies = true;
+ ManagedDBOptions dbOptions = new ManagedDBOptions();
+ dbOptions
+ .setIncreaseParallelism(Runtime.getRuntime().availableProcessors())
+ .setMaxBackgroundCompactions(maxBackgroundCompactions)
+ .setMaxBackgroundFlushes(maxBackgroundFlushes)
+ .setBytesPerSync(bytesPerSync)
+ .setCreateIfMissing(createIfMissing)
+ .setCreateMissingColumnFamilies(createMissingColumnFamilies);
+ return dbOptions;
+ }
+ return option;
+ }
+
+ @Override
+ public ManagedColumnFamilyOptions getColumnFamilyOptions(Path dbPath,
String cfName) {
+ ManagedColumnFamilyOptions option = null;
+ if (Objects.isNull(dbPath)) {
Review Comment:
The default dbPath is empty (not null). What happens when it's empty?
--
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-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
