refactor(inkless): cache LogConfig in InklessMetadataView#474
Merged
giuseppelillo merged 2 commits intomainfrom Mar 6, 2026
Merged
refactor(inkless): cache LogConfig in InklessMetadataView#474giuseppelillo merged 2 commits intomainfrom
giuseppelillo merged 2 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the MetadataView#getTopicConfig method to return LogConfig instead of Properties, consolidating configuration merging logic into the MetadataView implementation. This simplifies callers by removing redundant LogConfig.fromProps() calls and eliminates the need to separately access default configurations.
Key changes:
- Interface change in
MetadataViewto returnLogConfiginstead ofProperties - Implementation in
InklessMetadataViewnow merges default and topic-specific configs internally - Simplified caller code in
AppendHandlerandRetentionEnforcer
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| storage/inkless/src/main/java/io/aiven/inkless/control_plane/MetadataView.java | Updated interface method signature to return LogConfig instead of Properties |
| core/src/main/scala/kafka/server/metadata/InklessMetadataView.scala | Implemented getTopicConfig to merge default configs with topic overrides using LogConfig.fromProps |
| storage/inkless/src/main/java/io/aiven/inkless/produce/AppendHandler.java | Simplified getLogConfigs method by removing LogConfig.fromProps call and refactored constructor to accept Function<String, LogConfig> |
| storage/inkless/src/main/java/io/aiven/inkless/delete/RetentionEnforcer.java | Replaced LogConfig.fromProps call with direct method reference to getTopicConfig |
| storage/inkless/src/test/java/io/aiven/inkless/produce/AppendHandlerTest.java | Simplified test setup by removing SharedState and related mock dependencies |
| storage/inkless/src/test/java/io/aiven/inkless/delete/RetentionEnforcerTest.java | Updated mocks to return LogConfig instead of Properties and added try-with-resources for proper cleanup |
| storage/inkless/src/test/java/io/aiven/inkless/merge/FileMergerIntegrationTest.java | Simplified mock setup to return LogConfig directly |
| storage/inkless/src/test/java/io/aiven/inkless/delete/FileCleanerIntegrationTest.java | Simplified mock setup to return LogConfig directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/scala/kafka/server/metadata/InklessMetadataView.scala
Outdated
Show resolved
Hide resolved
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
from
1dce5f5 to
6e7b51f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
2 times, most recently
from
bb4745f to
5520b56
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
storage/inkless/src/main/java/io/aiven/inkless/produce/AppendHandler.java
Outdated
Show resolved
Hide resolved
storage/inkless/src/test/java/io/aiven/inkless/produce/AppendHandlerTest.java
Outdated
Show resolved
Hide resolved
jeqo
changed the title
Change MetadataView#getTopicConfig to return LogConfig instead of Properties, consolidating the responsibility of merging default and topic-specific configurations into the MetadataView implementation. This simplifies callers by: - Removing redundant LogConfig.fromProps() calls in RetentionEnforcer and AppendHandler - Eliminating the need for callers to access default configs separately - Reducing test setup by removing mock configuration for default configs The InklessMetadataView now handles merging default configs with topic overrides internally, providing a fully-resolved LogConfig to consumers.
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
from
f37c974 to
33c655a
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
from
33c655a to
f84843c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
from
f84843c to
0e445c5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Classic Kafka avoids reconstructing LogConfig on every produce request by caching it in LocalLog (via LogManager -> UnifiedLog -> LocalLog.config), updated through hooks in TopicConfigHandler and DynamicLogConfig. Inkless lacked an equivalent, calling LogConfig.fromProps on every getTopicConfig — an expensive operation that parses 40+ config fields. Add a ConcurrentHashMap cache in InklessMetadataView, populated lazily and kept up to date by: - TopicConfigHandler.updateLogConfig: pushes topic-level config changes - DynamicInklessLogConfig (new BrokerReconfigurable): rebuilds all cached entries when broker-level defaults change, preserving topic-specific overrides Also removes getDefaultConfig and updateTopicConfig from MetadataView interface — both are implementation details of InklessMetadataView. ReplicaManager now exposes the concrete InklessMetadataView type.
jeqo
force-pushed
the
jeqo/refactor-metadata-logconfig
branch
from
0e445c5 to
6ed00cb
Compare
AnatolyPopov
pushed a commit
that referenced
this pull request
Mar 23, 2026
* refactor(inkless): return LogConfig from MetadataView#getTopicConfig Change MetadataView#getTopicConfig to return LogConfig instead of Properties, consolidating the responsibility of merging default and topic-specific configurations into the MetadataView implementation. This simplifies callers by: - Removing redundant LogConfig.fromProps() calls in RetentionEnforcer and AppendHandler - Eliminating the need for callers to access default configs separately - Reducing test setup by removing mock configuration for default configs The InklessMetadataView now handles merging default configs with topic overrides internally, providing a fully-resolved LogConfig to consumers. * refactor(inkless): cache LogConfig per topic in InklessMetadataView Classic Kafka avoids reconstructing LogConfig on every produce request by caching it in LocalLog (via LogManager -> UnifiedLog -> LocalLog.config), updated through hooks in TopicConfigHandler and DynamicLogConfig. Inkless lacked an equivalent, calling LogConfig.fromProps on every getTopicConfig — an expensive operation that parses 40+ config fields. Add a ConcurrentHashMap cache in InklessMetadataView, populated lazily and kept up to date by: - TopicConfigHandler.updateLogConfig: pushes topic-level config changes - DynamicInklessLogConfig (new BrokerReconfigurable): rebuilds all cached entries when broker-level defaults change, preserving topic-specific overrides Also removes getDefaultConfig and updateTopicConfig from MetadataView interface — both are implementation details of InklessMetadataView. ReplicaManager now exposes the concrete InklessMetadataView type. (cherry picked from commit 590936e)
jeqo
added a commit
that referenced
this pull request
Mar 23, 2026
* refactor(inkless): return LogConfig from MetadataView#getTopicConfig Change MetadataView#getTopicConfig to return LogConfig instead of Properties, consolidating the responsibility of merging default and topic-specific configurations into the MetadataView implementation. This simplifies callers by: - Removing redundant LogConfig.fromProps() calls in RetentionEnforcer and AppendHandler - Eliminating the need for callers to access default configs separately - Reducing test setup by removing mock configuration for default configs The InklessMetadataView now handles merging default configs with topic overrides internally, providing a fully-resolved LogConfig to consumers. * refactor(inkless): cache LogConfig per topic in InklessMetadataView Classic Kafka avoids reconstructing LogConfig on every produce request by caching it in LocalLog (via LogManager -> UnifiedLog -> LocalLog.config), updated through hooks in TopicConfigHandler and DynamicLogConfig. Inkless lacked an equivalent, calling LogConfig.fromProps on every getTopicConfig — an expensive operation that parses 40+ config fields. Add a ConcurrentHashMap cache in InklessMetadataView, populated lazily and kept up to date by: - TopicConfigHandler.updateLogConfig: pushes topic-level config changes - DynamicInklessLogConfig (new BrokerReconfigurable): rebuilds all cached entries when broker-level defaults change, preserving topic-specific overrides Also removes getDefaultConfig and updateTopicConfig from MetadataView interface — both are implementation details of InklessMetadataView. ReplicaManager now exposes the concrete InklessMetadataView type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Return LogConfig from MetadataView#getTopicConfig instead of Properties, consolidating the LogConfig.fromProps merge logic into InklessMetadataView and removing it from callers (AppendHandler, RetentionEnforcer). Remove getDefaultConfig from the MetadataView interface as it was only needed by callers to construct LogConfig themselves.
Cache LogConfig per topic in InklessMetadataView to avoid reconstructing it on every produce request. Classic Kafka caches LogConfig in LocalLog (via LogManager -> UnifiedLog -> LocalLog.config), updated through hooks in TopicConfigHandler and DynamicLogConfig. Inkless lacked an equivalent — LogConfig.fromProps was called on every getTopicConfig, parsing and validating 40+ config fields each time.
The cache (ConcurrentHashMap) is populated lazily and kept up to date by:
Test plan: