fix(inkless): refresh cached topic config for diskless topics with a local log [KC-388] - #736
Merged
Merged
Conversation
…local log TopicConfigHandler refreshed InklessMetadataView's cached LogConfig only when this broker held no local log for the topic, using "no local log" as a proxy for "diskless topic". Consolidation broke the proxy: a consolidated topic is diskless and remote-storage backed, so it takes the classic makeLeader branch and every replica has a local log; a topic switched from classic keeps its pre-switch data on disk and has one too. On those topics the diskless produce and retention paths kept serving whatever config was resolved on first access, so a raised max.message.bytes had no effect (produces rejected with MESSAGE_TOO_LARGE against the old limit) and retention.ms/retention.bytes/cleanup.policy changes were silently ignored. The guard did not buy what it claimed: updateTopicConfig is computeIfPresent, so lazy population is preserved without it. Call it unconditionally. Observed in aiven-core CI kafka-service #26252 (ts-unification integration run), where raising a consolidated topic's max.message.bytes had no effect on produces. The unit tests now drive TopicConfigHandler against a real InklessMetadataView, so they assert on the config the diskless paths would read rather than on the handler's calls. testClassicTopicConfigUpdateDoesNotCallInklessMetadataView asserted the opposite of the fixed behavior and is replaced by one asserting what matters: no cache entry is created for a topic no diskless path has read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes stale InklessMetadataView cached LogConfig for diskless topics that still have local logs (e.g., consolidated diskless topics and classic→diskless switched topics), ensuring runtime topic-config changes (notably max.message.bytes and retention settings) take effect in diskless produce/retention paths.
Changes:
- Remove the “no local logs” gate so
TopicConfigHandleralways triggers anInklessMetadataView.updateTopicConfigrefresh (while preserving lazy population viacomputeIfPresent). - Rework unit coverage to assert observable
InklessMetadataViewbehavior (cached config values) rather than handler call patterns, including diskless-with-local-log scenarios. - Add an integration regression test ensuring raised
max.message.bytesbecomes effective on a consolidated diskless topic after initial production.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/main/scala/kafka/server/ConfigHandler.scala | Always forwards topic config changes to InklessMetadataView so cached diskless LogConfig entries refresh regardless of local-log presence. |
| core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala | Updates tests to use a real InklessMetadataView and validate cache refresh + lazy population semantics across diskless/local-log and classic topics. |
| core/src/test/java/kafka/server/InklessConsolidatedDisklessTopicsTest.java | Adds a regression test verifying a raised max.message.bytes is honored on an already-produced consolidated diskless topic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeqo
marked this pull request as ready for review
August 10, 2026 11:41
viktorsomogyi
approved these changes
Aug 10, 2026
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.
Left over from pre-consolidation Diskless topics.
TopicConfigHandlerrefreshed InklessMetadataView's cached LogConfig only when this broker held no local log for the topic, using "no local log" as a proxy for "diskless topic".Consolidation broke the proxy: a consolidated topic is diskless and remote-storage backed, so it takes the classic
makeLeaderbranch and every replica has a local log; a topic switched from classic keeps its pre-switch data on disk and has one too. On those topics the diskless produce and retention paths kept serving whatever config was resolved on first access, so a raised max.message.bytes had no effect (produces rejected with MESSAGE_TOO_LARGE against the old limit) andretention.ms/retention.bytes/cleanup.policychanges were silently ignored.The guard did not buy what it claimed:
updateTopicConfigiscomputeIfPresent, so lazy population is preserved without it. Call it unconditionally.Observed in aiven-core CI kafka-service #26252 (ts-unification integration run), where raising a consolidated topic's
max.message.byteshad no effect on produces.The unit tests now drive
TopicConfigHandleragainst a realInklessMetadataView, so they assert on the config the diskless paths would read rather than on the handler's calls.testClassicTopicConfigUpdateDoesNotCallInklessMetadataViewasserted the opposite of the fixed behavior and is replaced by one asserting what matters: no cache entry is created for a topic no diskless path has read.