tkalkirill commented on code in PR #1952:
URL: https://github.com/apache/ignite-3/pull/1952#discussion_r1182265533


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/gc/MvGc.java:
##########
@@ -221,34 +222,62 @@ private void scheduleGcForStorage(TablePartitionId 
tablePartitionId) {
                 return;
             }
 
-            if (storageHandler.gcInProgressFuture.get() != future) {
+            if (storageHandler.gcInProgressFuture.get() != currentGcFuture) {
                 // Someone in parallel is already collecting garbage, we will 
try once again after completion of gcInProgressFuture.
                 return;
             }
 
             try {
-                for (int i = 0; i < GC_BATCH_SIZE; i++) {
-                    HybridTimestamp lowWatermark = lowWatermarkReference.get();
+                HybridTimestamp lowWatermark = lowWatermarkReference.get();
 
-                    assert lowWatermark != null : tablePartitionId;
+                assert lowWatermark != null : tablePartitionId;
 
-                    // If storage has been deleted or there is no garbage, 
then for now we will stop collecting garbage for this storage.
-                    if 
(!storageHandlerByPartitionId.containsKey(tablePartitionId)
-                            || 
!storageHandler.storageUpdateHandler.vacuum(lowWatermark)) {
-                        return;
-                    }
-                }
-            } catch (Throwable t) {
-                future.completeExceptionally(t);
+                // If the storage has been deleted, then garbage collection is 
no longer necessary.
+                if 
(!storageHandlerByPartitionId.containsKey(tablePartitionId)) {
+                    currentGcFuture.complete(null);
 
-                return;
-            } finally {
-                if (!future.isCompletedExceptionally()) {
-                    future.complete(null);
+                    return;
                 }
-            }
 
-            scheduleGcForStorage(tablePartitionId);
+                StorageUpdateHandler storageUpdateHandler = 
storageHandler.storageUpdateHandler;
+
+                // We can only start garbage collection when the partition 
safe time is reached.
+                storageUpdateHandler.getSafeTimeTracker()
+                        .waitFor(lowWatermark)
+                        .thenApplyAsync(unused -> {
+                            for (int i = 0; i < GC_BATCH_SIZE; i++) {
+                                // If the storage has been deleted or there is 
no garbage, then we will stop.
+                                if 
(!storageHandlerByPartitionId.containsKey(tablePartitionId)
+                                        || 
!storageUpdateHandler.vacuum(lowWatermark)) {
+                                    return false;
+                                }
+                            }
+
+                            return true;
+                        }, executor)
+                        .whenComplete((isLeftGarbage, throwable) -> {

Review Comment:
   fix it



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