Repository navigation
fix(inkless:config): require consolidation for the classic-to-diskless switch [KC-596] - #822
Merged
viktorsomogyi merged 1 commit intoOct 13, 2026
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The added documentation contains a time-relative qualifier that violates the repository’s prose lint rules.
1 open finding
What changed in this PR
Requires classic-to-diskless switching to enable consolidation, preventing unsupported node configurations.
Changes:
- Reverses the configuration dependency and adds validation coverage.
- Updates affected tests, examples, and documentation.
| File | Description |
|---|---|
tests/kafkatest/tests/inkless/inkless_topic_switch_test.py |
Enables consolidation in switch tests. |
server-common/src/main/java/org/apache/kafka/server/config/ServerConfigs.java |
Updates configuration documentation. |
docs/inkless/DISKLESS_CONSOLIDATION.md |
Documents the reversed dependency. |
docker/examples/docker-compose-files/inkless/README.md |
Lists consolidation in the example. |
docker/examples/docker-compose-files/inkless/docker-compose.ts-unification.yml |
Enables consolidation in Docker. |
core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala |
Removes an obsolete comment. |
core/src/test/scala/unit/kafka/log/LogConfigTest.scala |
Permits consolidation without switching. |
core/src/test/java/kafka/server/InklessTopicTypeSwitcherClusterTest.java |
Updates switch test configuration. |
core/src/test/java/kafka/server/InklessConsolidatedDisklessReassignmentTest.java |
Removes an obsolete dependency comment. |
core/src/test/java/kafka/server/InklessConfigsTest.java |
Tests the new dependency direction. |
core/src/test/java/kafka/server/DisklessAndRemoteStorageConfigsTest.java |
Updates dependency explanation. |
core/src/main/scala/kafka/server/KafkaConfig.scala |
Enforces consolidation for switching. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…s switch Inkless does not support a switched topic that does not consolidate. Before this change, `diskless.remote.storage.consolidation.enable` required `diskless.allow.from.classic.enable`, but the switch did not require consolidation. A node with the switch on and consolidation off accepted a classic-to-diskless switch and enabled remote storage for the topic, and nothing consolidated it. The same configuration accepted a new diskless topic with remote storage that nothing consolidated. Neither state is supported at any rollout stage. Reverse the dependency in `KafkaConfig.validateValues`. `diskless.allow.from.classic.enable=true` now requires `diskless.remote.storage.consolidation.enable=true`, which still requires `diskless.managed.rf.enable` and `remote.log.storage.system.enable`. Consolidation without the switch stays valid. The switch flag is now the control that turns switching off. A node with the switch on and consolidation off fails at startup with: `requirement failed: diskless.allow.from.classic.enable requires diskless.remote.storage.consolidation.enable=true` The check runs on each node. A controller with the switch on and brokers with consolidation off can still produce the unsupported state. A cluster-wide check is a follow-up. Testing The new rule has no `KafkaConfig.fromProps` unit test. `InklessConfigsTest.allowFromClassicRequiresConsolidation` covers it through a cluster start and asserts the error message. `consolidationWithoutAllowFromClassicSucceeds` covers the valid reverse case. The tests for the removed switch prerequisites are dropped, because consolidation enforces the same prerequisites and its tests remain. The cluster and system tests that used the old combination (`InklessTopicTypeSwitcherClusterTest`, `inkless_topic_switch_test.py`, and the docker `ts-unification` example) now set consolidation together with the switch. Assisted-by: Claude Sonnet 5.5
jeqo
force-pushed
the
jeqo/kc-596-topic-switch-depend-consolidation
branch
from
October 12, 2026 13:42
d04f270 to
a43b72e
Compare
jeqo
marked this pull request as ready for review
October 12, 2026 15:17
viktorsomogyi
approved these changes
Oct 13, 2026
Contributor
There was a problem hiding this comment.
One general comment is that there will be a gap during genbump if controllers are bumped first as they'll carry these settings, while brokers won't. I don't think we can do much about this with reasonable effort, and after the genbump everything should work fine. Therefore this may be a non-issue.
viktorsomogyi
deleted the
jeqo/kc-596-topic-switch-depend-consolidation
branch
October 13, 2026 14:49
jeqo
added a commit
that referenced
this pull request
Oct 19, 2026
…s switch (#822) Inkless does not support a switched topic that does not consolidate. Before this change, `diskless.remote.storage.consolidation.enable` required `diskless.allow.from.classic.enable`, but the switch did not require consolidation. A node with the switch on and consolidation off accepted a classic-to-diskless switch and enabled remote storage for the topic, and nothing consolidated it. The same configuration accepted a new diskless topic with remote storage that nothing consolidated. Neither state is supported at any rollout stage. Reverse the dependency in `KafkaConfig.validateValues`. `diskless.allow.from.classic.enable=true` now requires `diskless.remote.storage.consolidation.enable=true`, which still requires `diskless.managed.rf.enable` and `remote.log.storage.system.enable`. Consolidation without the switch stays valid. The switch flag is now the control that turns switching off. A node with the switch on and consolidation off fails at startup with: `requirement failed: diskless.allow.from.classic.enable requires diskless.remote.storage.consolidation.enable=true` The check runs on each node. A controller with the switch on and brokers with consolidation off can still produce the unsupported state. A cluster-wide check is a follow-up. Testing The new rule has no `KafkaConfig.fromProps` unit test. `InklessConfigsTest.allowFromClassicRequiresConsolidation` covers it through a cluster start and asserts the error message. `consolidationWithoutAllowFromClassicSucceeds` covers the valid reverse case. The tests for the removed switch prerequisites are dropped, because consolidation enforces the same prerequisites and its tests remain. The cluster and system tests that used the old combination (`InklessTopicTypeSwitcherClusterTest`, `inkless_topic_switch_test.py`, and the docker `ts-unification` example) now set consolidation together with the switch. Assisted-by: Claude Sonnet 5.5
jeqo
added a commit
that referenced
this pull request
Oct 19, 2026
…s switch (#822) Inkless does not support a switched topic that does not consolidate. Before this change, `diskless.remote.storage.consolidation.enable` required `diskless.allow.from.classic.enable`, but the switch did not require consolidation. A node with the switch on and consolidation off accepted a classic-to-diskless switch and enabled remote storage for the topic, and nothing consolidated it. The same configuration accepted a new diskless topic with remote storage that nothing consolidated. Neither state is supported at any rollout stage. Reverse the dependency in `KafkaConfig.validateValues`. `diskless.allow.from.classic.enable=true` now requires `diskless.remote.storage.consolidation.enable=true`, which still requires `diskless.managed.rf.enable` and `remote.log.storage.system.enable`. Consolidation without the switch stays valid. The switch flag is now the control that turns switching off. A node with the switch on and consolidation off fails at startup with: `requirement failed: diskless.allow.from.classic.enable requires diskless.remote.storage.consolidation.enable=true` The check runs on each node. A controller with the switch on and brokers with consolidation off can still produce the unsupported state. A cluster-wide check is a follow-up. Testing The new rule has no `KafkaConfig.fromProps` unit test. `InklessConfigsTest.allowFromClassicRequiresConsolidation` covers it through a cluster start and asserts the error message. `consolidationWithoutAllowFromClassicSucceeds` covers the valid reverse case. The tests for the removed switch prerequisites are dropped, because consolidation enforces the same prerequisites and its tests remain. The cluster and system tests that used the old combination (`InklessTopicTypeSwitcherClusterTest`, `inkless_topic_switch_test.py`, and the docker `ts-unification` example) now set consolidation together with the switch. Assisted-by: Claude Sonnet 5.5
jeqo
added a commit
that referenced
this pull request
Oct 19, 2026
…s switch (#822) Inkless does not support a switched topic that does not consolidate. Before this change, `diskless.remote.storage.consolidation.enable` required `diskless.allow.from.classic.enable`, but the switch did not require consolidation. A node with the switch on and consolidation off accepted a classic-to-diskless switch and enabled remote storage for the topic, and nothing consolidated it. The same configuration accepted a new diskless topic with remote storage that nothing consolidated. Neither state is supported at any rollout stage. Reverse the dependency in `KafkaConfig.validateValues`. `diskless.allow.from.classic.enable=true` now requires `diskless.remote.storage.consolidation.enable=true`, which still requires `diskless.managed.rf.enable` and `remote.log.storage.system.enable`. Consolidation without the switch stays valid. The switch flag is now the control that turns switching off. A node with the switch on and consolidation off fails at startup with: `requirement failed: diskless.allow.from.classic.enable requires diskless.remote.storage.consolidation.enable=true` The check runs on each node. A controller with the switch on and brokers with consolidation off can still produce the unsupported state. A cluster-wide check is a follow-up. Testing The new rule has no `KafkaConfig.fromProps` unit test. `InklessConfigsTest.allowFromClassicRequiresConsolidation` covers it through a cluster start and asserts the error message. `consolidationWithoutAllowFromClassicSucceeds` covers the valid reverse case. The tests for the removed switch prerequisites are dropped, because consolidation enforces the same prerequisites and its tests remain. The cluster and system tests that used the old combination (`InklessTopicTypeSwitcherClusterTest`, `inkless_topic_switch_test.py`, and the docker `ts-unification` example) now set consolidation together with the switch. Assisted-by: Claude Sonnet 5.5
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 appl
2E85
ied 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.
Inkless does not support a switched topic that does not consolidate. Before this change,
diskless.remote.storage.consolidation.enablerequireddiskless.allow.from.classic.enable, but the switch did not require consolidation. A node with the switch on and consolidation off accepted a classic-to-diskless switch and enabled remote storage for the topic, and nothing consolidated it. The same configuration accepted a new diskless topic with remote storage that nothing consolidated. Neither state is supported at any rollout stage.Reverse the dependency in
KafkaConfig.validateValues.diskless.allow.from.classic.enable=truenow requiresdiskless.remote.storage.consolidation.enable=true, which still requiresdiskless.managed.rf.enableandremote.log.storage.system.enable. Consolidation without the switch stays valid. The switch flag is now the control that turns switching off.A node with the switch on and consolidation off fails at startup with:
requirement failed: diskless.allow.from.classic.enable requires diskless.remote.storage.consolidation.enable=trueThe check runs on each node. A controller with the switch on and brokers with consolidation off can still produce the unsupported state. A cluster-wide check is a follow-up.
Testing
The new rule has no
KafkaConfig.fromPropsunit test.InklessConfigsTest.allowFromClassicRequiresConsolidationcovers it through a cluster start and asserts the error message.consolidationWithoutAllowFromClassicSucceedscovers the valid reverse case. The tests for the removed switch prerequisites are dropped, because consolidation enforces the same prerequisites and its tests remain. The cluster and system tests that used the old combination (InklessTopicTypeSwitcherClusterTest,inkless_topic_switch_test.py, and the dockerts-unificationexample) now set consolidation together with the switch.KC-596