feat(inkless): POD-2386 Allow remote.storage.enabled when diskless is enabled by viktorsomogyi · Pull Request #556 · aiven/inkless

viktorsomogyi · 2026-03-30T09:01:25Z

Adds the diskless.remote.storage.consolidation.enable config. When it is enabled, it will

allow setting diskless.enable and remote.storage.enable on new topics
allow setting remote.storage.enable on existing diskless topics (where diskless.enable is enabled)
Setting remote.storag.enable on a diskless topic will be the intended way to start the consolidation of WAL segments into tiered topics.

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

server-common/src/main/java/org/apache/kafka/server/config/ServerConfigs.java

core/src/main/scala/kafka/server/KafkaConfig.scala

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

# Conflicts: # core/src/test/scala/unit/kafka/log/LogConfigTest.scala # storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java # Conflicts: # storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java # Conflicts: # storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

jeqo · 2026-04-01T15:52:36Z

core/src/main/scala/kafka/server/KafkaConfig.scala

+        s"${ServerConfigs.DISKLESS_REMOTE_STORAGE_CONSOLIDATION_ENABLE_CONFIG} requires ${ServerConfigs.DISKLESS_MANAGED_REPLICAS_ENABLE_CONFIG}=true")
+      require(remoteLogManagerConfig.isRemoteStorageSystemEnabled,
+        s"${ServerConfigs.DISKLESS_REMOTE_STORAGE_CONSOLIDATION_ENABLE_CONFIG} requires ${RemoteLogManagerConfig.REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP}=true")
+    }


agree it does not require allow-from-classic migration. Even though the topic type switcher work includes foundational work for TS consolidation (e.g. diskless start offset, etc) these are not guard by the feature flag.

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

jeqo

Looking good. Just a few more suggestions. Let's also ensure CI is passing.

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

jeqo · 2026-04-02T12:54:32Z

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java


    public static void validateTurningOffRemoteStorageWithDelete(Map<?, ?> newConfigs, boolean wasRemoteLogEnabled, boolean isRemoteLogStorageEnabled) {
        boolean isRemoteLogDeleteOnDisable = (Boolean) Utils.castToStringObjectMap(newConfigs).getOrDefault(TopicConfig.REMOTE_LOG_DELETE_ON_DISABLE_CONFIG, false);
        if (wasRemoteLogEnabled && !isRemoteLogStorageEnabled && !isRemoteLogDeleteOnDisable) {


We need to catch this no-op "config remains unchanged" pattern in the validations, e.g. in this case wasRemoteLogEnabled && !isRemoteLogStorageEnabled.
So, when validating diskless enforcements, they should only trigger when there is an actual change.

This can be useful for tooling like Terraform that always send the whole set of configs.

Added a number of test cases in LogConfigTest and changed the validation slightly to satisfy these test conditions.
Let me know if you need other cases covered too.

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io> Co-authored-by: Viktor Somogyi-Vass <viktorsomogyi@gmail.com>

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

jeqo

A last round of suggestions to make the conditions on LogConfigHelper more explicit. Apart from this it looks good to me.

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

jeqo

Forgot to publish this one 😅 should be the last one

jeqo · 2026-04-07T14:57:06Z

core/src/test/scala/unit/kafka/log/LogConfigTest.scala

+  }
+
+  @Test
+  def test_disklessAllowFromClassic_and_remoteStorageConsolidation_atUpdate(): Unit = {


nit: naming inconsistency with other tests

Suggested change

def test_disklessAllowFromClassic_and_remoteStorageConsolidation_atUpdate(): Unit = {

def testDisklessAllowFromClassicAndRemoteStorageConsolidationAtUpdate(): Unit = {

viktorsomogyi changed the title ~~feat(inkless): Add TS Consolidation configs~~ feat(inkless): Allow remote.storage.enabled when diskless too is enabled Mar 30, 2026

viktorsomogyi marked this pull request as ready for review March 30, 2026 13:29

viktorsomogyi changed the title ~~feat(inkless): Allow remote.storage.enabled when diskless too is enabled~~ feat(inkless): Allow remote.storage.enabled when diskless is enabled Mar 30, 2026

viktorsomogyi requested review from giuseppelillo and jeqo March 30, 2026 13:34

viktorsomogyi force-pushed the svv/ts-unification-configs branch from 506e6e1 to a699d79 Compare March 31, 2026 09:36

jeqo reviewed Mar 31, 2026

View reviewed changes

viktorsomogyi marked this pull request as draft March 31, 2026 14:37

jeqo mentioned this pull request Apr 1, 2026

fix(inkless:migration): fix classic-to-diskless migration validation #558

Merged

viktorsomogyi added 2 commits April 1, 2026 14:56

address some review comments, add tests

fd66674

viktorsomogyi force-pushed the svv/ts-unification-configs branch from a699d79 to cbb9eca Compare April 1, 2026 13:52

viktorsomogyi marked this pull request as ready for review April 1, 2026 13:53

viktorsomogyi force-pushed the svv/ts-unification-configs branch 2 times, most recently from c574bdb to 5786342 Compare April 1, 2026 14:17

address the rest of the review comments

12a8902

viktorsomogyi force-pushed the svv/ts-unification-configs branch from 5786342 to 12a8902 Compare April 1, 2026 14:19

jeqo requested changes Apr 1, 2026

View reviewed changes

core/src/test/scala/unit/kafka/log/LogConfigTest.scala Outdated Show resolved Hide resolved

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java Outdated Show resolved Hide resolved

jeqo reviewed Apr 1, 2026

View reviewed changes

Apply suggestions from code review

9aacc8b

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

viktorsomogyi requested a review from jeqo April 2, 2026 07:59

jeqo reviewed Apr 2, 2026

View reviewed changes

viktorsomogyi changed the title ~~feat(inkless): Allow remote.storage.enabled when diskless is enabled~~ feat(inkless): POD-2386 Allow remote.storage.enabled when diskless is enabled Apr 2, 2026

viktorsomogyi commented Apr 2, 2026

View reviewed changes

core/src/test/scala/unit/kafka/log/LogConfigTest.scala Outdated Show resolved Hide resolved

Apply suggestions from code review

7a7e04d

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io> Co-authored-by: Viktor Somogyi-Vass <viktorsomogyi@gmail.com>

jeqo reviewed Apr 2, 2026

View reviewed changes

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java Outdated Show resolved Hide resolved

viktorsomogyi and others added 4 commits April 2, 2026 16:58

Apply suggestions from code review

aaa1466

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

Add new test cases to verify mutual setting

605bf3a

Add validation logic to satisfy test cases

9811437

Fix unit test failures

8e350c0

jeqo reviewed Apr 7, 2026

View reviewed changes

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java Outdated Show resolved Hide resolved

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java Show resolved Hide resolved

jeqo reviewed Apr 7, 2026

View reviewed changes

storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java Outdated Show resolved Hide resolved

viktorsomogyi and others added 2 commits April 7, 2026 16:31

Apply suggestions from code review

1a17eb3

Co-authored-by: Jorge Esteban Quilcate Otoya <jorge.quilcate@aiven.io>

Remove unused method and fix carriage returns

f9b6a6a

jeqo reviewed Apr 7, 2026

View reviewed changes

jeqo approved these changes Apr 7, 2026

View reviewed changes

	def test_disklessAllowFromClassic_and_remoteStorageConsolidation_atUpdate(): Unit = {
	def testDisklessAllowFromClassicAndRemoteStorageConsolidationAtUpdate(): Unit = {

Conversation

viktorsomogyi commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeqo Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

jeqo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeqo Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

viktorsomogyi Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeqo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeqo left a comment

Choose a reason for hiding this comment

Uh oh!

jeqo Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

viktorsomogyi commented Mar 30, 2026 •

edited

Loading