carp84 commented on a change in pull request #15016:
URL: https://github.com/apache/flink/pull/15016#discussion_r598873871
##########
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:
I agree it is somewhat over-designed here but the original thought is to
try not to fail the test if `assumeTrue(incrementalCleanupSupported())` returns
`true` for RocksDB backend in the future.
The suggested way works but the name `isCurrentStateTableNull` is
misleading... It's "the state of the mapping for the composite of active key
and given namespace" (referring to `StateTable#get(N namespace)`) that should
be null, rather than the current state table. `isOriginalCleared` may be a
better name.
--
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:
[email protected]
