kamalcph commented on PR #15523: URL: https://github.com/apache/kafka/pull/15523#issuecomment-1996675858
Thanks for the review! > If the cleaner thread wasn't even started, are our really testing correct behaviour? For example, when we validate that cleaner shouldn't have deleted something, we would happily think that cleaner ran but didn't delete stuff whereas in reality, it may never have run at all. Should we instead add a wait loop in our setup for cache in the test to ensure that cleaner starts running? The RemoteIndexCache (RIC) and RemoteIndexCacheTest (RICT) are in different packages, so we cannot introduce a new package-private method in the RIC to overwrite it in RICT to ensure that the cleaner-thread started. Also, the ShutdownableThread is widely used by various module, so not changing the behavior of the thread. 1. The ShutdownableThread source code is in Java, so thought to write the new unit tests in java. 2. In this test, we don't expect any entries to expire. The test fails 2% of time which mean the remaining 98% of time, the cleaner-thread ran. In the assertion, we ensure that the cleaner-thread is closing correctly for both the cases. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
