+
Skip to content

feat(inkless): add persistent storage for infinispan #353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jeqo
Copy link
Contributor

@jeqo jeqo commented Jul 15, 2025

Enable Infinispan passivation feature that permits to use local disk as a second tier for cache entries.
By default, it reuses the log directory, so there is no need to create another volume and reuses the same storage as classic kafka. It adds a couple of knobs: lifespan and maxIdle to be used as a proxy on the amount of space cache entries could use.

@jeqo jeqo requested a review from Copilot July 15, 2025 09:21
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables Infinispan passivation to disk as a second-tier cache by introducing persistence flags and TTL controls, wiring a shared log directory through the stack, and updating tests and core components to match the new API.

  • Expose cache.persistence.enable, lifespan.sec, and max.idle.sec configs with defaults and getters.
  • Extend SharedState.initialize and BrokerServer/LogManager to accept a Path logDir and configure InfinispanCache accordingly.
  • Add helper cachePersistenceDir, update InfinispanCache constructor to apply passivation settings, and adjust tests for the new Path logDir parameter.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
storage/inkless/src/main/java/io/aiven/inkless/config/InklessConfig.java Added persistence configs, docs, default values, and getters.
storage/inkless/src/main/java/io/aiven/inkless/common/SharedState.java Extended initialize signature to include Path logDir and pass through to cache.
storage/inkless/src/main/java/io/aiven/inkless/cache/InfinispanCache.java Updated constructor for disk passivation, expiration settings, and added cachePersistenceDir helper.
storage/inkless/src/main/java/io/aiven/inkless/cache/ObjectCache.java Defined INKLESS_CACHE_DIR_NAME constant.
core/src/main/scala/kafka/server/BrokerServer.scala Passed Path.of(config.logDirs().get(0)) into SharedState.initialize.
core/src/main/scala/kafka/log/LogManager.scala Ignored the inkless cache directory during log scanning.
storage/inkless/src/test/java/io/aiven/inkless/merge/FileMergerMockedTest.java Added @TempDir Path logDir injection; missing Path import.
storage/inkless/src/test/java/io/aiven/inkless/merge/FileMergerIntegrationTest.java Added @TempDir Path logDir injection; missing Path import.
Comments suppressed due to low confidence (2)

storage/inkless/src/test/java/io/aiven/inkless/merge/FileMergerMockedTest.java:30

  • Missing import for java.nio.file.Path required by the @TempDir Path logDir field. Please add import java.nio.file.Path;.
import org.junit.jupiter.api.io.TempDir;

storage/inkless/src/test/java/io/aiven/inkless/merge/FileMergerIntegrationTest.java:42

  • Missing import for java.nio.file.Path required by the @TempDir Path logDir field. Please add import java.nio.file.Path;.
import org.junit.jupiter.api.io.TempDir;

@jeqo jeqo marked this pull request as ready for review July 15, 2025 09:29
@jeqo jeqo requested review from ivanyu and gharris1727 July 15, 2025 09:43
Copy link
Contributor

@gharris1727 gharris1727 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to the experiments with a bigger cache!

@@ -78,9 +78,24 @@ public class InklessConfig extends AbstractConfig {
private static final int CONSUME_CACHE_BLOCK_BYTES_DEFAULT = 16 * 1024 * 1024; // 16 MiB

public static final String CONSUME_CACHE_MAX_COUNT_CONFIG = CONSUME_PREFIX + "cache.max.count";
private static final String CONSUME_CACHE_MAX_COUNT_DOC = "The maximum number of objects to cache in memory.";
private static final String CONSUME_CACHE_MAX_COUNT_DOC = "The maximum number of objects to cache in memory. " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad config, counting number of objects is worse than counting memory.
Here or in a follow-up we should switch to max-size rather than max-count, but that might need us to change encoding/marshalling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. When using size-based config, cache creation fails with:

ISPN000504: Size (bytes) based eviction needs either off-heap or a binary compatible storage configured in the cache encoding

So, I think you're right; this will require further changes to be supported. Let's look into this in a follow-up

@jeqo jeqo force-pushed the jeqo/infinispan-persistent-storage branch from cad29ea to b836644 Compare July 16, 2025 07:40
Enable Infinispan passivation feature that permits to use local disk as
a second tier for cache entries.
By default, it reuses the log directory, so there is no need to create
another volume and reuses the same storage as classic kafka.
It adds a couple of knobs: lifespan and maxIdle to be used as a proxy on
the amount of space cache entries could use.
@jeqo jeqo force-pushed the jeqo/infinispan-persistent-storage branch 4 times, most recently from 4d2a5a5 to 0847d93 Compare July 16, 2025 10:18
@jeqo jeqo requested a review from gharris1727 July 16, 2025 10:31
jeqo added 3 commits July 16, 2025 13:56
- Moving DIR_NAME to InklessCache
- Undo LogManager import reordering
- Fix metrics NPE on cache.close
- Rename the time-based configs to expiration instead of persistence
- Move the expiration setup out of the persistence, to always apply
@jeqo jeqo force-pushed the jeqo/infinispan-persistent-storage branch from 6c0a6a1 to 90b02dc Compare July 16, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载