Myasuka commented on a change in pull request #15016: URL: https://github.com/apache/flink/pull/15016#discussion_r603034801
########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/state/ttl/TtlStateTestBase.java ########## @@ -471,6 +473,26 @@ public void testRestoreTtlAndRegisterNonTtlStateCompatFailure() throws Exception sbetc.createState(ctx().createStateDescriptor(), ""); } + @Test + public void testIncrementalCleanupWholeState() throws Exception { + assumeTrue(incrementalCleanupSupported()); + initTest(getConfBuilder(TTL).cleanupIncrementally(5, true).build()); + timeProvider.time = 0; + // create enough keys to trigger incremental rehash + updateKeys(0, INC_CLEANUP_ALL_KEYS, ctx().updateEmpty); + // expire all state + timeProvider.time = 120; + // trigger state clean up + for (int i = 0; i < INC_CLEANUP_ALL_KEYS; i++) { + sbetc.setCurrentKey(Integer.toString(i)); + } + // check all state cleaned up + for (int i = 0; i < INC_CLEANUP_ALL_KEYS; i++) { + sbetc.setCurrentKey(Integer.toString(i)); + assertTrue("Original state should be cleared", ctx().isOriginalNull()); Review comment: My original point is that we could avoid to introduce those `isValueOfCurrentKeyNull` related methods by just introduce one single `isCurrentStateTableNull` method within `TtlStateTestBase.java` (no matter what name it is). FLINK-21413 is about to fix a performance problem to avoid many unnecessary empty hash map and state entry, instead of incorrect query result. This new added test should only targets for heap-based keyed state backend and we can safely cast to `AbstractHeapState` here. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org