tkalkirill commented on code in PR #6341:
URL: https://github.com/apache/ignite-3/pull/6341#discussion_r2242766022
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/gc/MvGcTest.java:
##########
@@ -374,19 +376,35 @@ void testInvokeVacuumOnlyAfterReachSafeTime() {
assertThat(invokeVacuumMethodFuture, willSucceedFast());
}
+ @Test
+ void testRemoveStorageWithSafeTimeUpdateStuck() {
+ var startAwaitSafeTimeFuture = new CompletableFuture<Void>();
+
+ GcUpdateHandler gcUpdateHandler =
createWithSafeTimeUpdateStuck(startAwaitSafeTimeFuture);
+ TablePartitionId tablePartitionId = createTablePartitionId();
+
+ gc.addStorage(tablePartitionId, gcUpdateHandler);
+
+ assertThat(lowWatermark.updateAndNotify(new HybridTimestamp(10, 10)),
willCompleteSuccessfully());
+ assertThat(startAwaitSafeTimeFuture, willCompleteSuccessfully());
+ assertThat(gc.removeStorage(tablePartitionId),
willCompleteSuccessfully());
+
+ verify(gcUpdateHandler, never()).vacuumBatch(any(), anyInt(),
anyBoolean());
Review Comment:
> What's the purpose of this test? It seems to verify that, if safe time
never gets reached, vacuum is not called. But this happened before your fix,
didn't it?
Yes, but if I roll back my changes, the `removeStorage` will stuck.
We also check here that if we get stuck waiting for a safe time and call
`removeStorage`, then not only will we successfully abort the garbage
collection for the storage, but we will also not attempt to do `vacuumBatch`,
as if we had simply completed the future of wait for a safe time.
> Should we, instead, make sure that the GC attempt what was waiting on the
safe time gets 'unblocked'? Maybe through LogInspector, if there is no other
way?
I think the current check is more than enough.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
