Skip to content

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

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

[fix][admin] Allow defaultNumPartitions for non-partitioned autoTopicCreation override#25163
ChimdumebiNebolisa wants to merge 5 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

Comment on lines +2470 to +2475
AutoTopicCreationOverride overridePolicy = AutoTopicCreationOverride.builder()
.allowAutoTopicCreation(true)
.topicType(TopicType.NON_PARTITIONED.toString())
.defaultNumPartitions(5)
.build();
admin.namespaces().setAutoTopicCreation(namespaceName, overridePolicy);

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.

shouldn't this fail?

@ChimdumebiNebolisa ChimdumebiNebolisa Jul 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated this to address the review comment. The acceptance test now uses 1, and I added a separate test confirming that 5 is rejected with HTTP 412. The targeted tests and quickCheck pass locally. CI is currently 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