hemantk-12 commented on code in PR #5035:
URL: https://github.com/apache/ozone/pull/5035#discussion_r1261681974
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java:
##########
@@ -20,24 +20,37 @@
import org.apache.hadoop.hdds.utils.db.managed.ManagedDBOptions;
import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksDB;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionTimeoutException;
import org.rocksdb.ColumnFamilyDescriptor;
import org.rocksdb.ColumnFamilyHandle;
import org.rocksdb.RocksDBException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.File;
import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
+
/**
* Temporary class to test snapshot diff functionality.
* This should be removed later.
*/
public final class RdbUtil {
+ static final Logger LOG =
+ LoggerFactory.getLogger(RdbUtil.class);
+ private static final Duration POLL_DELAY_DURATION = Duration.ZERO;
+ private static final Duration POLL_INTERVAL_DURATION =
Duration.ofMillis(100);
+ private static final Duration POLL_MAX_DURATION = Duration.ofSeconds(5);
Review Comment:
Please remove these constants and unused imports.
##########
hadoop-hdds/rocksdb-checkpoint-differ/pom.xml:
##########
@@ -82,6 +82,10 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-rocks-native</artifactId>
</dependency>
+ <dependency>
Review Comment:
Do we need this now?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java:
##########
@@ -123,7 +133,7 @@ public BackgroundTaskResult call() throws Exception {
long snapshotLimit = snapshotLimitPerTask;
- while (iterator.hasNext() && snapshotLimit > 0) {
+ while (iterator.hasNext() && snapshotLimit > 0 && running.get()) {
Review Comment:
I don't think if `running.get()` check is really needed because
[BackgroundService#shutdown()](https://github.com/apache/ozone/blob/a66e43e2838f087229cb4b0fac1e1dc969ab80dd/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/BackgroundService.java#L140)
makes sure that all the active threads get terminated. Same for overriding
`start()` and `shutdown()` functions.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3632,7 +3633,7 @@ TermIndex installCheckpoint(String leaderId, Path
checkpointLocation,
keyManager.stop();
stopSecretManager();
stopTrashEmptier();
-
+ omSnapshotManager.getSnapshotCache().invalidateAll();
Review Comment:
I see it solves the problem but ideally we should only invalidate cache when
[OmMetadataManager was
stopped](https://github.com/apache/ozone/blob/a66e43e2838f087229cb4b0fac1e1dc969ab80dd/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java#L3743)
because when `OmMetadataManager` gets stopped we load [everything including
cache
fresh](https://github.com/apache/ozone/blob/a66e43e2838f087229cb4b0fac1e1dc969ab80dd/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone)/om/OzoneManager.java#L3736C9-L3736C22).
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -80,6 +80,7 @@
import org.apache.hadoop.hdds.scm.ScmInfo;
import org.apache.hadoop.hdds.scm.client.HddsClientUtils;
import org.apache.hadoop.hdds.server.OzoneAdmins;
+import org.apache.hadoop.hdds.utils.BackgroundService;
Review Comment:
Remove this import.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java:
##########
@@ -34,6 +34,7 @@
import org.apache.hadoop.hdds.utils.db.managed.ManagedTransactionLogIterator;
import org.apache.hadoop.hdds.utils.db.managed.ManagedWriteBatch;
import org.apache.hadoop.hdds.utils.db.managed.ManagedWriteOptions;
+import org.apache.ozone.rocksdb.util.RdbUtil;
Review Comment:
Please remove these unused imports.
--
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]
