hudeqi commented on code in PR #14511:
URL: https://github.com/apache/kafka/pull/14511#discussion_r1359460675


##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -561,6 +561,108 @@ class RemoteIndexCacheTest {
     assertTrue(cache.internalCache().estimatedSize() == 0)
   }
 
+  @Test
+  def testCorrectnessForCacheAndIndexFilesWhenResizeCache(): Unit = {
+
+    def getIndexFileFromRemoteCacheDir(suffix: String) = {
+      try {
+        Files.walk(cache.cacheDir().toPath())
+          .filter(Files.isRegularFile(_))
+          .filter(path => path.getFileName.toString.endsWith(suffix))
+          .findAny()
+      } catch {
+        case _: NoSuchFileException => Optional.empty()
+      }
+    }
+
+    // The test process for resizing is: put 1 entry -> evict to empty -> put 
3 entries with limited capacity of 2 entries -> evict to 1 entry
+    val estimateEntryBytesSize = estimateOneEntryBytesSize()
+    val tpId = new TopicIdPartition(Uuid.randomUuid(), new 
TopicPartition("foo", 0))
+    val metadataList = generateRemoteLogSegmentMetadata(size = 3, tpId)
+
+    assertCacheSize(0)
+    // getIndex for first time will call rsm#fetchIndex
+    val cacheEntry = cache.getIndexEntry(metadataList.head)
+    assertCacheSize(1)
+    
assertTrue(getIndexFileFromRemoteCacheDir(LogFileUtils.INDEX_FILE_SUFFIX).isPresent)
+    
assertTrue(getIndexFileFromRemoteCacheDir(LogFileUtils.TXN_INDEX_FILE_SUFFIX).isPresent)
+    
assertTrue(getIndexFileFromRemoteCacheDir(LogFileUtils.TIME_INDEX_FILE_SUFFIX).isPresent)
+
+    // Reduce the cache size to 1 byte to ensure that all the entries are 
evicted from it.
+    cache.resizeCacheSize(1L)
+
+    // wait until entry is marked for deletion
+    TestUtils.waitUntilTrue(() => cacheEntry.isMarkedForCleanup,
+      "Failed to mark cache entry for cleanup after resizing cache.")
+    TestUtils.waitUntilTrue(() => cacheEntry.isCleanStarted,
+      "Failed to cleanup cache entry after resizing cache.")
+
+    // verify no index files on remote cache dir
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(LogFileUtils.INDEX_FILE_SUFFIX).isPresent,
+      s"Offset index file should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(LogFileUtils.TXN_INDEX_FILE_SUFFIX).isPresent,
+      s"Txn index file should not be present on disk at ${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(LogFileUtils.TIME_INDEX_FILE_SUFFIX).isPresent,
+      s"Time index file should not be present on disk at ${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(LogFileUtils.DELETED_FILE_SUFFIX).isPresent,
+      s"Index file marked for deletion should not be present on disk at 
${cache.cacheDir()}")
+
+    assertTrue(cache.internalCache().estimatedSize() == 0)
+
+    // Increase cache capacity to only store 2 entries
+    cache.resizeCacheSize(2 * estimateEntryBytesSize)
+    assertCacheSize(0)
+
+    val entry0 = cache.getIndexEntry(metadataList(0))
+    val entry1 = cache.getIndexEntry(metadataList(1))
+    cache.getIndexEntry(metadataList(2))
+    assertCacheSize(2)
+    val missingMetadata = metadataList(0)
+    val missingEntry = entry0
+    // wait until evicted entry is marked for deletion
+    TestUtils.waitUntilTrue(() => missingEntry.isMarkedForCleanup,
+      "Failed to mark evicted cache entry for cleanup after resizing cache.")
+    TestUtils.waitUntilTrue(() => missingEntry.isCleanStarted,
+      "Failed to cleanup evicted cache entry after resizing cache.")
+    // verify no index files for `missingEntry` on remote cache dir
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteOffsetIndexFileName(missingMetadata)).isPresent,
+      s"Offset index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteTimeIndexFileName(missingMetadata)).isPresent,
+      s"Time index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteTransactionIndexFileName(missingMetadata)).isPresent,
+      s"Txn index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteDeletedSuffixIndexFileName(missingMetadata)).isPresent,
+      s"Index file marked for deletion for evicted entry should not be present 
on disk at ${cache.cacheDir()}")
+
+    // Reduce cache capacity to only store 1 entries
+    cache.resizeCacheSize(1 * estimateEntryBytesSize)
+    assertCacheSize(1)
+
+    val nextMissingMetadata = metadataList(1)
+    val nextMissingEntry = entry1
+    // wait until evicted entry is marked for deletion
+    TestUtils.waitUntilTrue(() => nextMissingEntry.isMarkedForCleanup,
+      "Failed to mark evicted cache entry for cleanup after resizing cache.")
+    TestUtils.waitUntilTrue(() => nextMissingEntry.isCleanStarted,
+      "Failed to cleanup evicted cache entry after resizing cache.")
+    // verify no index files for `nextMissingEntry` on remote cache dir
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteOffsetIndexFileName(nextMissingMetadata)).isPresent,
+      s"Offset index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteTimeIndexFileName(nextMissingMetadata)).isPresent,
+      s"Time index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteTransactionIndexFileName(nextMissingMetadata)).isPresent,
+      s"Txn index file for evicted entry should not be present on disk at 
${cache.cacheDir()}")
+    TestUtils.waitUntilTrue(() => 
!getIndexFileFromRemoteCacheDir(remoteDeletedSuffixIndexFileName(nextMissingMetadata)).isPresent,
+      s"Index file marked for deletion for evicted entry should not be present 
on disk at ${cache.cacheDir()}")

Review Comment:
   updated.



-- 
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

Reply via email to