8000 fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false by viktorsomogyi · Pull Request #804 · aiven/inkless · GitHub
[go: up one dir, main page]

Skip to content

fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false - #804

Merged
giuseppelillo merged 2 commits into
mainfrom
svv/fix-tsu-config
Sep 24, 2026
Merged

giuseppelillo merged 2 commits into
mainfrom
svv/fix-tsu-config

Conversation

@viktorsomogyi
Copy link
Copy Markdown
Contributor

The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires.

Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Deleting an enabled remote-storage override can bypass the consolidation invariant.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Allows routine configuration updates for legacy diskless topics with persisted remote.storage.enable=false.

Changes:

  • Grandfathers unchanged diskless/remote-disabled configurations.
  • Prioritizes the consolidation-specific validation error.
  • Updates unit and integration expectations.
File summaries
File Description
storage/.../LogConfig.java Refines transition validation.
core/.../LogConfigTest.scala Expands transition tests.
core/.../DisklessAndRemoteStorageConfigsTest.java Updates integration assertions.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ote.storage.enable=false

The controller validates the merged topic config map, so an existing
remote.storage.enable=false override looked like a disable on every
alter. Reject only new diskless + remote=false transitions, and do that
before mutual exclusion so the specific invariant still fires.

Co-authored-by: Cursor <cursoragent@cursor.com>
A deleted remote.storage.enable override disappears from the resulting
config map. Resolve the target state from the combined config so
incremental deletion and legacy omission cannot leave a diskless topic
without remote storage.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The validation change and its regression coverage were fully reviewed with no unresolved issues.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@giuseppelillo
giuseppelillo merged commit d16ea42 into main Sep 24, 2026
7 checks passed
@giuseppelillo
giuseppelillo deleted the svv/fix-tsu-config branch September 24, 2026 13:38
giuseppelillo pushed a commit that referenced this pull request Sep 24, 2026
…ote.storage.enable=false (#804)

* fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false

The controller validates the merged topic config map, so an existing
remote.storage.enable=false override looked like a disable on every
alter. Reject only new diskless + remote=false transitions, and do that
before mutual exclusion so the specific invariant still fires.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(inkless:config): reject remote storage override deletion

A deleted remote.storage.enable override disappears from the resulting
config map. Resolve the target state from the combined config so
incremental deletion and legacy omission cannot leave a diskless topic
without remote storage.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
giuseppelillo pushed a commit that referenced this pull request Sep 24, 2026
…ote.storage.enable=false (#804)

* fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false

The controller validates the merged topic config map, so an existing
remote.storage.enable=false override looked like a disable on every
alter. Reject only new diskless + remote=false transitions, and do that
before mutual exclusion so the specific invariant still fires.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(inkless:config): reject remote storage override deletion

A deleted remote.storage.enable override disappears from the resulting
config map. Resolve the target state from the combined config so
incremental deletion and legacy omission cannot leave a diskless topic
without remote storage.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
giuseppelillo pushed a commit that referenced this pull request Sep 24, 2026
…ote.storage.enable=false (#804)

* fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false

The controller validates the merged topic config map, so an existing
remote.storage.enable=false override looked like a disable on every
alter. Reject only new diskless + remote=false transitions, and do that
before mutual exclusion so the specific invariant still fires.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(inkless:config): reject remote storage override deletion

A deleted remote.storage.enable override disappears from the resulting
config map. Resolve the target state from the combined config so
incremental deletion and legacy omission cannot leave a diskless topic
without remote storage.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0