-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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
, andmax.idle.sec
configs with defaults and getters. - Extend
SharedState.initialize
andBrokerServer/LogManager
to accept aPath logDir
and configureInfinispanCache
accordingly. - Add helper
cachePersistenceDir
, updateInfinispanCache
constructor to apply passivation settings, and adjust tests for the newPath 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 addimport 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 addimport java.nio.file.Path;
.
import org.junit.jupiter.api.io.TempDir;
There was a problem hiding this 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!
storage/inkless/src/main/java/io/aiven/inkless/cache/ObjectCache.java
Outdated
Show resolved
Hide resolved
storage/inkless/src/main/java/io/aiven/inkless/cache/InfinispanCache.java
Show resolved
Hide resolved
@@ -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. " + |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
storage/inkless/src/main/java/io/aiven/inkless/config/InklessConfig.java
Outdated
Show resolved
Hide resolved
storage/inkless/src/main/java/io/aiven/inkless/cache/InfinispanCache.java
Show resolved
Hide resolved
storage/inkless/src/main/java/io/aiven/inkless/cache/InfinispanCache.java
Outdated
Show resolved
Hide resolved
storage/inkless/src/main/java/io/aiven/inkless/config/InklessConfig.java
Outdated
Show resolved
Hide resolved
storage/inkless/src/main/java/io/aiven/inkless/config/InklessConfig.java
Outdated
Show resolved
Hide resolved
cad29ea
to
b836644
Compare
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.
4d2a5a5
to
0847d93
Compare
- 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
- Clarifies min 10sec for lifespan
6c0a6a1
to
90b02dc
Compare
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.