smengcl commented on code in PR #5104:
URL: https://github.com/apache/ozone/pull/5104#discussion_r1279539839
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -171,13 +173,10 @@ private void
generateSnapshotCheckpoint(HttpServletRequest request,
LOG.info("Received excluding SST {}", receivedSstList);
}
+ Path tmpdir = null;
try (BootstrapStateHandler.Lock lock = getBootstrapStateLock().lock()) {
- if (dbStore.getRocksDBCheckpointDiffer() != null) {
- dbStore.getRocksDBCheckpointDiffer().incrementTarballRequestCount();
- }
-
- checkpoint = dbStore.getCheckpoint(flush);
-
+ tmpdir = Files.createTempDirectory("tmpBootstrapData");
Review Comment:
nit
```suggestion
tmpdir = Files.createTempDirectory("om-bootstrap-data");
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DBCheckpointServlet.java:
##########
@@ -171,13 +173,10 @@ private void
generateSnapshotCheckpoint(HttpServletRequest request,
LOG.info("Received excluding SST {}", receivedSstList);
}
+ Path tmpdir = null;
try (BootstrapStateHandler.Lock lock = getBootstrapStateLock().lock()) {
- if (dbStore.getRocksDBCheckpointDiffer() != null) {
- dbStore.getRocksDBCheckpointDiffer().incrementTarballRequestCount();
- }
-
- checkpoint = dbStore.getCheckpoint(flush);
-
+ tmpdir = Files.createTempDirectory("tmpBootstrapData");
+ checkpoint = getCheckpoint(tmpdir, flush);
Review Comment:
There is an implication when RocksDB checkpoint dir is set like this, IMO.
Since `tmpdir` would be a temp working dir under `/tmp/`, and that `/tmp/`
is commonly a `tmpfs` mount (backed by system memory) in various Linux distros,
it differs from the filesystem, say `ext4`, that the active RocksDB uses. Thus,
RocksDB won't be able to hardlink the SSTs and had to fall back to copying
files.
So there are two concerns:
1. it could increase the time every DB checkpoint operation takes by quite a
bit
2. `/tmp/` might not be able to hold the entire Ozone RocksDB
Maybe it is better to use the default path instead for DB checkpoints?
--
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]
