rkhachatryan commented on code in PR #19441:
URL: https://github.com/apache/flink/pull/19441#discussion_r853867505


##########
flink-dstl/flink-dstl-dfs/src/test/java/org/apache/flink/changelog/fs/ChangelogStorageMetricsTest.java:
##########
@@ -295,6 +349,39 @@ public void close() {
         }
     }
 
+    private static class FixedLatencyUploader implements StateChangeUploader {
+        private final long latency;
+
+        public FixedLatencyUploader(long latency) {
+            this.latency = latency;
+        }
+
+        @Override
+        public UploadTasksResult upload(Collection<UploadTask> tasks) throws 
IOException {
+            Map<UploadTask, Map<StateChangeSet, Long>> map = new HashMap<>();
+
+            try {
+                TimeUnit.MILLISECONDS.sleep(latency);

Review Comment:
   Thanks for the explanation. 
   In other words, `testAttemptsPerUpload` completes the last attempt and 
`testTotalAttemptsPerUpload` completes the first attempt, right?
   
   IMO it's better to keep these tests separate, because the scenarios and the 
assertions are different.
   
   As for the implementation of `testTotalAttemptsPerUpload`, how about waiting 
in the 1st attempt for the others to fail?
   Something like `CountDownLatch` or `CompletableFuture` + `AtomicInteger` ?



-- 
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]

Reply via email to