feat(inkless): Seal the local log when a topic is migrated from classic to diskless by giuseppelillo · Pull Request #533 · aiven/inkless

giuseppelillo · 2026-03-12T14:46:31Z

Sealing is applied for both existing leaders and newly elected leaders.

…ic to diskless Sealing is applied for both existing leaders and newly elected leaders.

jeqo

Looks solid. Just a minor suggestion.

jeqo · 2026-03-12T21:06:10Z

core/src/main/scala/kafka/cluster/Partition.scala

  ): LogAppendInfo = {
    val (info, leaderHWIncremented) = inReadLock(leaderIsrUpdateLock) {
+      if (_sealed) {
+        throw new NotLeaderOrFollowerException(


Worth noting here that this exception is used on purpose to force clients to refresh metadata and retry (as this is not explicit from the code)

Right it's not very useful to request clients to refresh metadata. I've changed to BrokerNotAvailableException so that clients can retry but they are not forced to refresh the metadata.

Right. It's not ideal, but that's the only way of keeping the client retrying until migration completes IIUC. Otherwise it fails at the client side, and they crash.
In the expected scenario, these round-trips should give enough room for migration to complete.

There's also this aspect to consider: #533 (comment)

But I would not change the catch logic. So we either force a metadata refresh for every produce request or we accept that there will be failures in metrics during a migration. I lean towards the latter option.

…titions

AnatolyPopov

LGTM! Thanks, looks exactly as we discussed 👍 But leaving without approve for review from others.

Copilot

Pull request overview

This PR adds “sealing” behavior to prevent any further local-log writes when a topic transitions from classic to diskless, covering both existing leaders (at the time the config flips) and partitions that (re)become leaders while diskless is enabled.

Changes:

Add sealing detection in BrokerMetadataPublisher when diskless.enable flips to true.
Add ReplicaManager.sealTopicPartitions plus logic to seal partitions during leader transitions for diskless topics.
Introduce a sealed flag in Partition that rejects appends; add unit tests validating sealing behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
core/src/main/scala/kafka/server/metadata/BrokerMetadataPublisher.scala	Detects classic→diskless via topic config delta and seals existing leaders.
core/src/main/scala/kafka/server/ReplicaManager.scala	Adds sealing helper and seals partitions when applying leader deltas for diskless topics.
core/src/main/scala/kafka/cluster/Partition.scala	Adds sealed state, seal API, and rejects `appendRecordsToLeader` when sealed.
core/src/test/scala/unit/kafka/server/metadata/BrokerMetadataPublisherTest.scala	Tests sealing is triggered on `diskless.enable` config change.
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala	Tests `sealTopicPartitions` and sealing during `applyDelta`.
core/src/test/scala/unit/kafka/cluster/PartitionTest.scala	Tests `Partition.seal` behavior and append rejection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

core/src/main/scala/kafka/cluster/Partition.scala

core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala

1. Add partition to set of changed partitions also when it's sealed 2. Log the sealing only when it actually happens 3. Fix test for correct follower->leader scenario

…rtitions

jeqo · 2026-03-13T11:21:55Z

core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala


    // TODO: Add more fetch tests combinations, edge cases ara not covered yet.

+  @Test


Should we move these new tests into @Nested class Inkless instead?

They already are, but the indentation of this test was wrong, fixed it now.

Right! indentation trick me. Thanks for fixing it!

jeqo

LGTM, thanks @giuseppelillo!

As discussed internally, we can add metrics for sealed partitions in a follow-up 👍🏽

…ic to diskless (#533) Sealing is applied for both existing leaders and newly elected leaders. (cherry picked from commit d9bf220) # Conflicts: # core/src/main/scala/kafka/server/ReplicaManager.scala # core/src/test/scala/unit/kafka/cluster/PartitionTest.scala

…ic to diskless (#533) Sealing is applied for both existing leaders and newly elected leaders.

…ic to diskless (#533) Sealing is applied for both existing leaders and newly elected leaders. (cherry picked from commit d9bf220) # Conflicts: # core/src/main/scala/kafka/server/ReplicaManager.scala # core/src/test/scala/unit/kafka/cluster/PartitionTest.scala

giuseppelillo force-pushed the giuseppelillo/diskless-migration-sealing branch 2 times, most recently from 4e56634 to fc4d461 Compare March 12, 2026 15:33

feat(inkless): Seal the local log when a topic is migrated from class…

4e41e53

…ic to diskless Sealing is applied for both existing leaders and newly elected leaders.

giuseppelillo force-pushed the giuseppelillo/diskless-migration-sealing branch from fc4d461 to 4e41e53 Compare March 12, 2026 15:56

giuseppelillo changed the title ~~feat(inkless): seal the log when diskless is enabled~~ feat(inkless): Seal the local log when a topic is migrated from classic to diskless Mar 12, 2026

giuseppelillo marked this pull request as ready for review March 12, 2026 16:27

jeqo reviewed Mar 12, 2026

View reviewed changes

fixup! Throw BrokerNotAvailableException when producing on sealed par…

80bc894

…titions

giuseppelillo requested a review from Copilot March 13, 2026 08:37

Copilot started reviewing on behalf of giuseppelillo March 13, 2026 08:37 View session

AnatolyPopov reviewed Mar 13, 2026

View reviewed changes

Copilot AI reviewed Mar 13, 2026

View reviewed changes

giuseppelillo added 2 commits March 13, 2026 10:30

fixup! Add suggestions by Copilot

cb5db62

1. Add partition to set of changed partitions also when it's sealed 2. Log the sealing only when it actually happens 3. Fix test for correct follower->leader scenario

fixup! Throw ReplicaNotAvailableException when producing on sealed pa…

f422c9b

…rtitions

jeqo reviewed Mar 13, 2026

View reviewed changes

fixup! Fix indentation in test

18e69ae

jeqo approved these changes Mar 13, 2026

View reviewed changes

jeqo merged commit d9bf220 into main Mar 13, 2026
5 checks passed

jeqo deleted the giuseppelillo/diskless-migration-sealing branch March 13, 2026 12:47

jeqo pushed a commit that referenced this pull request Mar 23, 2026

feat(inkless): Seal the local log when a topic is migrated from class…

87dc5cb

…ic to diskless (#533) Sealing is applied for both existing leaders and newly elected leaders.


		// TODO: Add more fetch tests combinations, edge cases ara not covered yet.

		@Test

Conversation

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

Uh oh!

Uh oh!

jeqo left a comment

Choose a reason for hiding this comment

Uh oh!

jeqo Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

giuseppelillo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

giuseppelillo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

AnatolyPopov left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeqo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

giuseppelillo Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jeqo Mar 13, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

giuseppelillo commented Mar 12, 2026 •

edited

Loading

jeqo Mar 13, 2026 •

edited

Loading