rpuch commented on code in PR #6341:
URL: https://github.com/apache/ignite-3/pull/6341#discussion_r2242714170
##########
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 was already happening before your
fix, didn't it?
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?
##########
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 was already happening before your
fix, wasn't it?
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?
--
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
