adoroszlai commented on code in PR #11317:
URL: https://github.com/apache/ozone/pull/11317#discussion_r4096119183
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java:
##########
@@ -259,6 +264,47 @@ public void testRocksDBCheckpointCleanup() throws
Exception {
checkpoint.getCheckpointLocation()));
}
+ @Test
+ public void testCheckpointAfterManagerClose(@TempDir File cpDir) throws
Exception {
+ insertRandomData(rdbStore, 1);
+ RDBCheckpointManager manager = new RDBCheckpointManager(rdbStore.getDb(),
"test");
+ manager.close();
+
+ RocksDBCheckpoint checkpoint =
manager.createCheckpoint(cpDir.getAbsolutePath());
+ assertNotNull(checkpoint);
+ assertTrue(Files.exists(checkpoint.getCheckpointLocation()));
Review Comment:
nit:
```suggestion
assertThat(checkpoint.getCheckpointLocation()).exists();
```
(also elsewhere)
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java:
##########
@@ -239,34 +239,15 @@ public boolean isClosed() {
return isClosed.get();
}
- /**
- * Represents a checkpoint of the db.
- *
- * @see ManagedCheckpoint
- */
- final class RocksCheckpoint implements Closeable {
- private final ManagedCheckpoint checkpoint;
-
- private RocksCheckpoint() {
- this.checkpoint = ManagedCheckpoint.create(db);
- }
-
- public void createCheckpoint(Path path) throws RocksDatabaseException {
- try (UncheckedAutoCloseable ignored = acquire()) {
- checkpoint.get().createCheckpoint(path.toString());
- } catch (RocksDBException e) {
- closeOnError(e);
- throw toRocksDatabaseException(this, "createCheckpoint " + path, e);
- }
- }
-
- public long getLatestSequenceNumber() throws RocksDatabaseException {
- return RocksDatabase.this.getLatestSequenceNumber();
- }
-
- @Override
- public void close() throws RocksDatabaseException {
- checkpoint.close();
+ /** @return the latest sequence number after the checkpoint is created. */
+ long createCheckpoint(Path path) throws RocksDatabaseException {
+ try (UncheckedAutoCloseable ignored = acquire();
+ ManagedCheckpoint checkpoint = ManagedCheckpoint.create(db)) {
+ checkpoint.get().createCheckpoint(path.toString());
Review Comment:
nit: move `path.toString()` out of `try` (can also be reused in `catch`)
--
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]
