smengcl commented on code in PR #5014:
URL: https://github.com/apache/ozone/pull/5014#discussion_r1260261877
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java:
##########
@@ -251,13 +265,22 @@ private void processDir(Path dir, Set<Path> copyFiles,
LOG.debug("Skipping unneeded file: " + file);
continue;
}
- processDir(file, copyFiles, hardLinkFiles, toExcludeFiles,
- snapshotPaths, excluded);
+ if (!processDir(file, copyFiles, hardLinkFiles, toExcludeFiles,
+ snapshotPaths, excluded, copySize)) {
+ return false;
+ }
} else {
- processFile(file, copyFiles, hardLinkFiles, toExcludeFiles,
excluded);
+ long fileSize = processFile(file, copyFiles, hardLinkFiles,
+ toExcludeFiles, excluded);
+ if (copySize.get() + fileSize > maxTotalSstSize) {
+ return false;
Review Comment:
It looks like the default single SST file size limit is 64 MB:
https://github.com/apache/ozone/blob/275653e0a997852be44ed55ccd8dabcc255cb399/hadoop-hdds/container-service/src/test/resources/123-dn-container.db/OPTIONS-000036#L128
And that the level multiplier is 1 as well:
https://github.com/apache/ozone/blob/275653e0a997852be44ed55ccd8dabcc255cb399/hadoop-hdds/container-service/src/test/resources/123-dn-container.db/OPTIONS-000036#L109
which implies that the largest SST file currently in Ozone should be around
64 MB, according to the [tuning
guide](https://github.com/facebook/rocksdb/wiki/RocksDB-Tuning-Guide):
> target_file_size_base and target_file_size_multiplier -- Files in level 1
will have target_file_size_base bytes. Each next level's file size will be
target_file_size_multiplier bigger than previous one. However, by default
target_file_size_multiplier is 1, so files in all L1..Lmax levels are equal.
Increasing target_file_size_base will reduce total number of database files,
which is generally a good thing. We recommend setting target_file_size_base to
be max_bytes_for_level_base / 10, so that there are 10 files in level 1.
so while in theory this wouldn't be a problem if the
`ozone.om.ratis.snapshot.max.total.sst.size` config is set to 100 MB
(currently) because the largest SST file wouldn't exceed that, IMO it could
still be an issue when misconfigured or the RocksDB config is tuned in the
future.
@prashantpogde accidentally merged this PR. You may open an addendum PR for
the same jira (or filing a another PR will do as well).
--
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]
