Skip to content

[SS-74] Don't try creating a Kafka progress topic if it already exists.#35252

Open
ublubu wants to merge 2 commits intoMaterializeInc:mainfrom
ublubu:precreated-progress
Open

[SS-74] Don't try creating a Kafka progress topic if it already exists.#35252
ublubu wants to merge 2 commits intoMaterializeInc:mainfrom
ublubu:precreated-progress

Conversation

@ublubu
Copy link
Contributor

@ublubu ublubu commented Feb 27, 2026

It is an accepted pattern to create the topics needed for a new sink (e.g. as superuser) before actually creating the sink (as a less-privileged user).

Before this fix, Materialize always tries creating the progress topic, even if it already exists, leading to an authorization failure.
After this fix, Materialize sees that the progress topic exists, so it continues without attempting the operation that requires the "Create" ACL.

@ublubu ublubu requested review from a team and josharenberg February 27, 2026 16:41
@ublubu ublubu requested a review from a team as a code owner February 27, 2026 16:41
@github-actions
Copy link

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

Comment on lines +209 to +218
// First, check whether the topic already exists.
let metadata = client
.inner()
// N.B. It is extremely important not to ask specifically
// about the topic here, even though the API supports it!
// Asking about the topic will create it automatically...
// with the wrong number of partitions. Yes, this is
// unbelievably horrible.
.fetch_metadata(None, Some(Duration::from_secs(10)))?;
let already_exists = metadata.topics().iter().any(|t| t.name() == new_topic.name) || {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new part. The rest is just indented (inside the || { ... })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copypaste from https://github.com/ublubu/materialize/blob/588547c24336fc41d5039afce1f5e414a28356d5/src/kafka-util/src/admin.rs#L251-L263

let metadata = client
    .inner()
    // N.B. It is extremely important not to ask specifically
    // about the topic here, even though the API supports it!
    // Asking about the topic will create it automatically...
    // with the wrong number of partitions. Yes, this is
    // unbelievably horrible.
    .fetch_metadata(None, Some(Duration::from_secs(10)))?;
let topic = metadata
    .topics()
    .iter()
    .find(|t| t.name() == new_topic.name)
    .ok_or_else(|| anyhow!("unable to fetch topic metadata after creation"))?;

@ublubu ublubu force-pushed the precreated-progress branch from 5f5f328 to fe8bce7 Compare February 27, 2026 17:32
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.

1 participant