hemantk-12 commented on code in PR #4884:
URL: https://github.com/apache/ozone/pull/4884#discussion_r1234759874
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -494,6 +494,11 @@ private OMConfigKeys() {
public static final String OZONE_OM_SNAPSHOT_DIFF_REPORT_MAX_PAGE_SIZE
= "ozone.om.snapshot.diff.max.page.size";
+
+ public static final String OZONE_OM_SNAPSHOT_DB_MAX_OPEN_FILES
+ = "ozone.om.snapshot.db.max.open.files";
+ public static final int OZONE_OM_SNAPSHOT_DB_MAX_OPEN_FILES_DEFAULT
Review Comment:
QQ: Should we use -1 here just to make sure if it is not set -1 is used?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -380,7 +380,7 @@ private OmMetadataManagerImpl(OzoneConfiguration conf, File
dir, String name)
checkSnapshotDirExist(checkpoint);
}
setStore(loadDB(conf, metaDir, dbName, false,
- java.util.Optional.of(Boolean.TRUE), false, false));
+ java.util.Optional.of(Boolean.TRUE), Optional.of(maxOpenFiles),
false, false));
Review Comment:
I don't agree with overriding default with default. But it is OK if we want
to allow that.
Zero is still no handled properly.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -147,6 +149,7 @@ public final class OmSnapshotManager implements
AutoCloseable {
"snap-diff-purged-job-table";
private final long diffCleanupServiceInterval;
+ private final int maxOpenFilesSnapshotDB;
Review Comment:
nit:
```suggestion
private final int maxOpenSstFilesInSnapshotDb;
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBStoreBuilder.java:
##########
@@ -182,6 +183,17 @@ private void applyDBDefinition(DBDefinition definition) {
}
}
+ public DBStoreBuilder setMaxNumberOfOpenFiles(Integer maxNumberOfOpenFiles) {
+ this.maxNumberOfOpenFiles = maxNumberOfOpenFiles;
+ return this;
+ }
+
+ private void setDBOptionsProps(ManagedDBOptions dbOptions) {
Review Comment:
I think `setMaxOpenFiles` would be better naming. Naming `setDBOptionsProps`
is generic and it is not doing that.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -525,22 +525,24 @@ public void start(OzoneConfiguration configuration)
throws IOException {
public static DBStore loadDB(OzoneConfiguration configuration, File metaDir)
throws IOException {
return loadDB(configuration, metaDir, OM_DB_NAME, false,
- java.util.Optional.empty(), true, true);
+ java.util.Optional.empty(), Optional.empty(), true, true);
}
public static DBStore loadDB(OzoneConfiguration configuration, File metaDir,
String dbName, boolean readOnly,
java.util.Optional<Boolean>
- disableAutoCompaction)
+ disableAutoCompaction,
+ java.util.Optional<Integer> maxOpenFiles)
Review Comment:
I agree with @smengcl. Someone can set it to zero either by mistake or
unintentionally assuming 0 is default.
--
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]
