chungen0126 commented on code in PR #8173:
URL: https://github.com/apache/ozone/pull/8173#discussion_r2016231324
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeSchemaThreeDBDefinition.java:
##########
@@ -110,17 +114,19 @@ public DatanodeSchemaThreeDBDefinition(String dbPath,
DatanodeDBProfile dbProfile = DatanodeDBProfile
.getProfile(config.getEnum(HDDS_DB_PROFILE, HDDS_DEFAULT_DB_PROFILE));
+ Path pathToDb = Paths.get(
+ config.get(HddsConfigKeys.ROCKS_DB_CONFIG_PATH,
HddsConfigKeys.ROCKS_DB_CONFIG_PATH_DEFAULT));
ManagedColumnFamilyOptions cfOptions =
- dbProfile.getColumnFamilyOptions(config);
+ dbProfile.getColumnFamilyOptions(config, pathToDb,
DEFAULT_COLUMN_FAMILY_NAME);
Review Comment:
`cfOptions` is no longer used since we now use their respective options.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBConfigFromFile.java:
##########
@@ -106,38 +110,86 @@ public static String getOptionsFileNameFromDB(String
dbFileName) {
* control OzoneManager.db configs from a file, we need to create a file
* called OzoneManager.db.ini and place that file in $OZONE_DIR/etc/hadoop.
*
- * @param dbFileName - The DB File Name, for example, OzoneManager.db.
- * @param cfDescs - ColumnFamily Handles.
+ * @param dbPath - The DB File Name, for example, OzoneManager.db.
* @return DBOptions, Options to be used for opening/creating the DB.
* @throws IOException
*/
- public static ManagedDBOptions readFromFile(String dbFileName,
- List<ColumnFamilyDescriptor> cfDescs) throws IOException {
- Preconditions.checkNotNull(dbFileName);
- Preconditions.checkNotNull(cfDescs);
- Preconditions.checkArgument(!cfDescs.isEmpty());
+ public static ManagedDBOptions readDBOptionsFromFile(Path dbPath) throws
IOException {
+ List<ColumnFamilyDescriptor> descriptors = new ArrayList<>();
+ ManagedDBOptions readDBOptions = readFromFile(dbPath, descriptors);
+ //readDBOptions will be freed once the store using it is closed, but the
descriptors need to be closed.
+ closeDescriptors(descriptors);
+ return readDBOptions;
+ }
- //TODO: Add Documentation on how to support RocksDB Mem Env.
- Env env = Env.getDefault();
- ManagedDBOptions options = null;
- File configLocation = getConfigLocation();
- if (configLocation != null &&
- StringUtil.isNotBlank(configLocation.toString())) {
- Path optionsFile = Paths.get(configLocation.toString(),
- getOptionsFileNameFromDB(dbFileName));
-
- if (optionsFile.toFile().exists()) {
- options = new ManagedDBOptions();
- try {
- OptionsUtil.loadOptionsFromFile(optionsFile.toString(),
- env, options, cfDescs, true);
-
- } catch (RocksDBException rdEx) {
- throw toIOException("Unable to find/open Options file.", rdEx);
- }
+ public static ManagedColumnFamilyOptions readCFOptionsFromFile(Path dbPath,
String cfName) throws IOException {
+ List<ColumnFamilyDescriptor> descriptors = new ArrayList<>();
+ String validatedCfName = StringUtil.isEmpty(cfName) ?
StringUtils.bytes2String(DEFAULT_COLUMN_FAMILY) : cfName;
+ ManagedColumnFamilyOptions resultCfOptions = null;
+ try (ManagedDBOptions ignored = readFromFile(dbPath, descriptors)) {
+ ColumnFamilyDescriptor descriptor = descriptors.stream()
+ .filter(desc ->
StringUtils.bytes2String(desc.getName()).equals(validatedCfName))
+ .findAny().orElse(null);
+ if (descriptor != null) {
+ resultCfOptions = new
ManagedColumnFamilyOptions(descriptor.getOptions());
}
+ } finally {
+ closeDescriptors(descriptors);
+ }
+ return resultCfOptions;
+ }
+
+ private static ManagedDBOptions readFromFile(Path dbPath,
List<ColumnFamilyDescriptor> descriptors)
+ throws IOException {
+ Preconditions.checkNotNull(dbPath);
+
+ //TODO: Add Documentation on how to support RocksDB Mem Env.
+ Path generatedDBPath = generateDBPath(dbPath);
+ if (!generatedDBPath.toFile().exists()) {
+ LOG.warn("Error trying to read generated rocksDB file: {}, file does not
exists.", generatedDBPath);
+ return null;
+ }
+ ManagedDBOptions options = new ManagedDBOptions();
+ try (ManagedConfigOptions configOptions = new ManagedConfigOptions()) {
+ OptionsUtil.loadOptionsFromFile(configOptions,
generatedDBPath.toString(), options, descriptors);
+ } catch (RocksDBException rdEx) {
+ options.close();
+ closeDescriptors(descriptors);
Review Comment:
Closing descriptors here is duplicated with closing them in
`readCFOptionsFromFile`. Instead of closing them here close them in
readDBOptionsFromFile. As follows:
```
public static ManagedDBOptions readDBOptionsFromFile(Path dbPath) throws
IOException {
List<ColumnFamilyDescriptor> descriptors = new ArrayList<>();
try {
ManagedDBOptions readDBOptions = readFromFile(dbPath, descriptors);
return readDBOptions;
} finally {
//readDBOptions will be freed once the store using it is closed, but
the descriptors need to be closed.
closeDescriptors(descriptors);
}
}
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java:
##########
@@ -142,10 +134,6 @@ private DBStoreBuilder(ConfigurationSource configuration,
this.rocksDbStat = configuration.getTrimmed(
OZONE_METADATA_STORE_ROCKSDB_STATISTICS,
OZONE_METADATA_STORE_ROCKSDB_STATISTICS_DEFAULT);
- this.rocksDbCfWriteBufferSize = (long) configuration.getStorageSize(
Review Comment:
Since OZONE_METADATA_STORE_ROCKSDB_CF_WRITE_BUFFER_SIZE is no longer used,
the TestOMSnapshotDAG should also be rewritten. It is still using it.
In addition, I think we need a default ini file to set `write_buffer_size`
as the default size in Ozone is different from rocksdb.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -2680,6 +2680,14 @@
Absolute path to HDDS metadata dir.
</description>
</property>
+ <property>
+ <name>rocks.db.config.path</name>
+ <value/>
+ <tag>OZONE, OM</tag>
Review Comment:
`rocks.db.config.path` is also for SCM, Datanode. I think we should also add
SCM, CONTAINER, STORAGE to the tag as well.
Do you think we need to split `rocks.db.config.path` into
`scm.db.config.path`, `om.db.config.path` and `datanode.db.config.path` to
avoid potential bugs when running different components on the same machine?
--
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]
