Skip to content

[fix][admin] Allow defaultNumPartitions for non-partitioned autoTopicCreation override - #25163

Open
ChimdumebiNebolisa wants to merge 8 commits into
apache:masterfrom
ChimdumebiNebolisa:fix-24998-allow-defaultNumPartitions
Open

[fix][admin] Allow defaultNumPartitions for non-partitioned autoTopicCreation override#25163
ChimdumebiNebolisa wants to merge 8 commits into
apache:masterfrom
ChimdumebiNebolisa:fix-24998-allow-defaultNumPartitions

Conversation

@ChimdumebiNebolisa

@ChimdumebiNebolisa ChimdumebiNebolisa commented Jan 19, 2026

Copy link
Copy Markdown

Fixes #24998

Motivation

The namespace autoTopicCreation override validation rejects defaultNumPartitions whenever topicType=non-partitioned. This forces clients and infrastructure-as-code tools to conditionally omit the field, making shared configuration templates harder to maintain.

The relaxation is intentionally narrow. For non-partitioned overrides, defaultNumPartitions may be null, 0, or 1. Values greater than 1 remain invalid.

Accepted values are persisted as submitted but ignored during runtime topic creation. Auto-created topics therefore remain non-partitioned.

Modifications

  • Updated AutoTopicCreationOverrideImpl.validateOverride to allow defaultNumPartitions for NON_PARTITIONED only when the value is null, 0, or 1.
  • Added unit coverage confirming that 0 and 1 are accepted and 2 is rejected.
  • Added broker-level coverage confirming that:
    • an accepted value of 1 is persisted but ignored when the topic is auto-created
    • a value of 5 is rejected by the admin REST API with HTTP 412

Verifying this change

./gradlew :pulsar-common:test \
  --tests "org.apache.pulsar.common.policies.data.AutoTopicCreationOverrideTest" \
  -PtestRetryCount=0

./gradlew :pulsar-broker:test \
  --tests "org.apache.pulsar.broker.admin.AdminApi2Test.testAutoTopicCreationOverrideNonPartitionedDefaultNumPartitionsIgnored" \
  --tests "org.apache.pulsar.broker.admin.AdminApi2Test.testAutoTopicCreationOverrideNonPartitionedDefaultNumPartitionsRejected" \
  -PtestRetryCount=0

./gradlew quickCheck

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc-not-needed

@ChimdumebiNebolisa
ChimdumebiNebolisa force-pushed the fix-24998-allow-defaultNumPartitions branch from 6a10c6a to 7a13f42 Compare January 19, 2026 13:19
@ChimdumebiNebolisa

Copy link
Copy Markdown
Author

Updated validateOverride to keep topic type validation and allow defaultNumPartitions for NON_PARTITIONED only when null, 0, or 1. Updated unit tests to cover 0, 1, and reject 2.

@lhotari lhotari changed the title [fix][admin] Allow defaultNumPartitions for non-partitioned autoTopicCreation override (Fixes #24998) [fix][admin] Allow defaultNumPartitions for non-partitioned autoTopicCreation override Jan 20, 2026
@lhotari

lhotari commented Jan 20, 2026

Copy link
Copy Markdown
Member

Updated validateOverride to keep topic type validation and allow defaultNumPartitions for NON_PARTITIONED only when null, 0, or 1. Updated unit tests to cover 0, 1, and reject 2.

Please add this information to the PR description.

@lhotari lhotari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @ChimdumebiNebolisa

@lhotari lhotari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The updated tests now match the intended narrow validation: non-partitioned overrides accept null, 0, or 1, while the REST-level test verifies that 5 is rejected with HTTP 412. The accepted value is also verified as persisted but ignored at topic creation, producing a non-partitioned topic. I found no remaining issue in the current 125-line PR diff. The tracker delta contains unrelated upstream merges since the previous approval, so those were excluded in favor of the canonical three-file PR diff.

@ChimdumebiNebolisa

Copy link
Copy Markdown
Author

Fixed the Broker Group 3 CI failure in 1bdd1b6 by updating the stale validation-message assertion.

The exact failing test now passes locally, and git diff --check is clean.

The new CI runs are awaiting maintainer approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API usability: defaultNumPartitions validation is too strict for non-partitioned topic type

2 participants