[GitHub] [kafka] kamalcph commented on a diff in pull request #14381: KAFKA-14912:Add a dynamic config for remote index cache size

["kamalcph (via GitHub)" <gi...@apache.org>]

kamalcph commented on code in PR #14381:
URL: https://github.com/apache/kafka/pull/14381#discussion_r1324171502


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -79,6 +79,7 @@ public class RemoteIndexCache implements Closeable {
     private static final String TMP_FILE_SUFFIX = ".tmp";
     public static final String REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD = "remote-log-index-cleaner";
     public static final String DIR_NAME = "remote-log-index-cache";
+    private static final long DEFAULT_REMOTE_INDEX_CACHE_SIZE_BYTES = 1024 * 1024L;

Review Comment:
   Let's move this constant to `RemoteIndexCacheTest` since this is used only in the test.



##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -112,27 +115,55 @@ public class RemoteIndexCache implements Closeable {
      *
      * We use {@link Caffeine} cache instead of implementing a thread safe LRU cache on our own.
      */
-    private final Cache<Uuid, Entry> internalCache;
-    private final RemoteStorageManager remoteStorageManager;
-    private final ShutdownableThread cleanerThread;
+    private Cache<Uuid, Entry> internalCache;
 
+    // Visible for testing
     public RemoteIndexCache(RemoteStorageManager remoteStorageManager, String logDir) throws IOException {

Review Comment:
   We can remove this constructor and use the other constructor from the test.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org