ibessonov commented on code in PR #4161:
URL: https://github.com/apache/ignite-3/pull/4161#discussion_r1698012451


##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/engine/AbstractStorageEngineTest.java:
##########
@@ -288,6 +291,54 @@ void testIndexRecoveryForMultipleTables(@Mock 
StorageIndexDescriptorSupplier ind
         }
     }
 
+    /**
+     * Ensures that {@code flush(false)} does not trigger a flush explicitly, 
but only subscribes to the next scheduled one.
+     */
+    @Test
+    void testSubscribeToFlush() throws Exception {
+        assumeFalse(storageEngine.isVolatile());
+
+        int tableId = 1;
+        int lastAppliedIndex = 10;
+        int lastAppliedTerm = 20;
+
+        StorageTableDescriptor tableDescriptor = new 
StorageTableDescriptor(tableId, 1, DEFAULT_STORAGE_PROFILE);
+        StorageIndexDescriptorSupplier indexSupplier = 
mock(StorageIndexDescriptorSupplier.class);
+
+        MvTableStorage mvTableStorage = 
storageEngine.createMvTable(tableDescriptor, indexSupplier);
+
+        try (AutoCloseable ignored0 = mvTableStorage::close) {
+            CompletableFuture<MvPartitionStorage> mvPartitionStorageFuture = 
mvTableStorage.createMvPartition(0);
+
+            assertThat(mvPartitionStorageFuture, willCompleteSuccessfully());
+            MvPartitionStorage mvPartitionStorage = 
mvPartitionStorageFuture.join();
+
+            try (AutoCloseable ignored1 = mvPartitionStorage::close) {
+                assertThat(mvPartitionStorage.flush(), 
willCompleteSuccessfully());
+
+                {
+                    mvPartitionStorage.runConsistently(locker -> {
+                        mvPartitionStorage.lastApplied(lastAppliedIndex, 
lastAppliedTerm);
+
+                        return null;
+                    });
+
+                    CompletableFuture<Void> subscribeFuture1 = 
mvPartitionStorage.flush(false);
+                    assertThat(subscribeFuture1, willTimeoutFast());
+
+                    CompletableFuture<Void> subscribeFuture2 = 
mvPartitionStorage.flush(false);
+                    assertThat(subscribeFuture2, willTimeoutFast());
+
+                    assertSame(subscribeFuture1, subscribeFuture2);
+                    assertThat(mvPartitionStorage.flush(), 
willCompleteSuccessfully());
+
+                    assertTrue(subscribeFuture1.isDone());
+                    assertTrue(subscribeFuture2.isDone());

Review Comment:
   Please make sure that there's no data race here. Does this test fail if you 
run it 1000 times?
   What I mean: here you claim that `flush(false)` futures must be completed 
before `flush()` future is completed. Do we really guarantee that?



##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/engine/AbstractStorageEngineTest.java:
##########
@@ -288,6 +291,54 @@ void testIndexRecoveryForMultipleTables(@Mock 
StorageIndexDescriptorSupplier ind
         }
     }
 
+    /**
+     * Ensures that {@code flush(false)} does not trigger a flush explicitly, 
but only subscribes to the next scheduled one.
+     */
+    @Test
+    void testSubscribeToFlush() throws Exception {
+        assumeFalse(storageEngine.isVolatile());
+
+        int tableId = 1;
+        int lastAppliedIndex = 10;
+        int lastAppliedTerm = 20;
+
+        StorageTableDescriptor tableDescriptor = new 
StorageTableDescriptor(tableId, 1, DEFAULT_STORAGE_PROFILE);
+        StorageIndexDescriptorSupplier indexSupplier = 
mock(StorageIndexDescriptorSupplier.class);
+
+        MvTableStorage mvTableStorage = 
storageEngine.createMvTable(tableDescriptor, indexSupplier);
+
+        try (AutoCloseable ignored0 = mvTableStorage::close) {
+            CompletableFuture<MvPartitionStorage> mvPartitionStorageFuture = 
mvTableStorage.createMvPartition(0);
+
+            assertThat(mvPartitionStorageFuture, willCompleteSuccessfully());
+            MvPartitionStorage mvPartitionStorage = 
mvPartitionStorageFuture.join();
+
+            try (AutoCloseable ignored1 = mvPartitionStorage::close) {
+                assertThat(mvPartitionStorage.flush(), 
willCompleteSuccessfully());
+
+                {

Review Comment:
   This scope seems strange, what does it do?



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