adoroszlai commented on code in PR #4214:
URL: https://github.com/apache/ozone/pull/4214#discussion_r1088669532
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBCheckpointManager.java:
##########
@@ -96,6 +100,21 @@ public RocksDBCheckpoint createCheckpoint(String parentDir,
String name) {
return null;
}
+ /**
+ * Wait for checkpoint directory gets created for 10 secs.
+ * <p>
+ * By default, Awaitility waits for 10 seconds and then throw
+ * a ConditionTimeoutException if the condition has not been fulfilled.
+ * The default poll interval and poll delay is 100 milliseconds.
+ */
+ private void waitForCheckpointDirectoryExist(File file) throws IOException {
+ try {
+ await().until(file::exists);
+ } catch (ConditionTimeoutException exception) {
+ LOG.info("Checkpoint directory didn't get created in 10 secs.");
Review Comment:
Would it be useful to include the dir name in the log message?
##########
hadoop-hdds/framework/pom.xml:
##########
@@ -166,7 +166,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd";>
<artifactId>rocksdb-checkpoint-differ</artifactId>
<version>${hdds.version}</version>
</dependency>
-
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <version>4.2.0</version>
+ </dependency>
Review Comment:
When adding a new dependency, please:
1. update `hadoop-ozone/dist/src/main/license/bin/LICENSE.txt` (also for any
new transitive dependencies)
2. update `hadoop-ozone/dist/src/main/license/jar-report.txt` with
versionless jar names (also for any new transitive dependencies)
3. create a [property in root
POM](https://github.com/apache/ozone/blob/ec1e098694d019043f7340e06b54bfc57af6ed47/pom.xml#L139-L140)
for the version number
4. define the dependency in [root POM's
`dependencyManagement`](https://github.com/apache/ozone/blob/ec1e098694d019043f7340e06b54bfc57af6ed47/pom.xml#L756-L760)
section using the property
5. add it to the submodule POM(s) without version definition
```
Changed jars:
--- /dev/fd/63 2023-01-27
06:[10](https://github.com/hemantk-12/ozone/actions/runs/4021901854/jobs/6911260833#step:5:11):29.3776[12](https://github.com/hemantk-12/ozone/actions/runs/4021901854/jobs/6911260833#step:5:13)253
+0000
+++ /dev/fd/62
[20](https://github.com/hemantk-12/ozone/actions/runs/4021901854/jobs/6911260833#step:5:21)23-01-27
06:10:29.37761[22](https://github.com/hemantk-12/ozone/actions/runs/4021901854/jobs/6911260833#step:5:23)53
+0000
@@ -9,6 +9,7 @@
share/ozone/lib/asm.jar
share/ozone/lib/aspectjrt.jar
share/ozone/lib/aspectjweaver.jar
+share/ozone/lib/awaitility.jar
share/ozone/lib/aws-java-sdk-core.jar
share/ozone/lib/aws-java-sdk-kms.jar
share/ozone/lib/aws-java-sdk-s3.jar
@@ -60,6 +61,7 @@
share/ozone/lib/hadoop-hdfs.jar
share/ozone/lib/hadoop-shaded-guava.jar
share/ozone/lib/hadoop-shaded-protobuf_3_7.jar
+share/ozone/lib/hamcrest.jar
share/ozone/lib/hdds-annotation-processing.jar
share/ozone/lib/hdds-client.jar
share/ozone/lib/hdds-common.jar
```
https://github.com/hemantk-12/ozone/actions/runs/4021901854/jobs/6911260833#step:5:19
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBCheckpointManager.java:
##########
@@ -85,6 +87,8 @@ public RocksDBCheckpoint createCheckpoint(String parentDir,
String name) {
LOG.info("Created checkpoint at {} in {} milliseconds",
checkpointPath, duration);
Review Comment:
Should this "success" log and `end`/`duration` be moved into the new method,
after successful wait?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBCheckpointManager.java:
##########
@@ -85,6 +87,8 @@ public RocksDBCheckpoint createCheckpoint(String parentDir,
String name) {
LOG.info("Created checkpoint at {} in {} milliseconds",
checkpointPath, duration);
+ waitForCheckpointDirectoryExist(checkpointPath.toFile());
+
return new RocksDBCheckpoint(
checkpointPath,
currentTime,
Review Comment:
What if the wait times out? Is the checkpoint object being returned still
valid?
--
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]
