Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChimdumebiNebolisa wants to merge 5 commits into
base: master
Choose a base branch
from
+76 −4
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ecb222
Relax validation for defaultNumPartitions on non-partitioned autoTopi…
ChimdumebiNebolisa Jan 19, 2026
7a13f42
Add broker test: accept but ignore defaultNumPartitions for non-parti…
ChimdumebiNebolisa Jan 19, 2026
505efbe
Fix validation for defaultNumPartitions on non-partitioned override
ChimdumebiNebolisa Jan 19, 2026
c00cf68
Merge remote-tracking branch 'upstream/master' into fix-24998-allow-d…
ChimdumebiNebolisa Jul 11, 2026
95842a7
[fix][test] Align non-partitioned auto topic creation tests
ChimdumebiNebolisa Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2566,6 +2566,54 @@ public void testAutoTopicCreationOverrideWithMaxNumPartitionsLimit() throws Exce
assertTrue(ex.getCause() instanceof NotAcceptableException);
}
}

@Test
public void testAutoTopicCreationOverrideNonPartitionedDefaultNumPartitionsIgnored() throws Exception {
String tenantName = newUniqueName("prop-xyz2");
String namespaceName = tenantName + "/ns-" + System.currentTimeMillis();
TenantInfoImpl tenantInfo = new TenantInfoImpl(Set.of("role1", "role2"), Set.of("test"));
admin.tenants().createTenant(tenantName, tenantInfo);
admin.namespaces().createNamespace(namespaceName, Set.of("test"));

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

@lhotari lhotari Apr 27, 2026

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 •
edited
Loading

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.


AutoTopicCreationOverride storedPolicy = admin.namespaces().getAutoTopicCreation(namespaceName);
assertTrue(storedPolicy.isAllowAutoTopicCreation());
assertEquals(storedPolicy.getTopicType(), TopicType.NON_PARTITIONED.toString());
assertEquals(storedPolicy.getDefaultNumPartitions(), Integer.valueOf(1));

String topicName = "persistent://" + namespaceName + "/auto-topic-" + UUID.randomUUID();
pulsarClient.newProducer().topic(topicName).create().close();

assertEquals(admin.topics().getPartitionedTopicMetadata(topicName).partitions, 0);
}

@Test
public void testAutoTopicCreationOverrideNonPartitionedDefaultNumPartitionsRejected() throws Exception {
String tenantName = newUniqueName("prop-xyz2");
String namespaceName = tenantName + "/ns-" + System.currentTimeMillis();
TenantInfoImpl tenantInfo = new TenantInfoImpl(Set.of("role1", "role2"), Set.of("test"));
admin.tenants().createTenant(tenantName, tenantInfo);
admin.namespaces().createNamespace(namespaceName, Set.of("test"));

AutoTopicCreationOverride overridePolicy = AutoTopicCreationOverride.builder()
.allowAutoTopicCreation(true)
.topicType(TopicType.NON_PARTITIONED.toString())
.defaultNumPartitions(5)
.build();
try {
admin.namespaces().setAutoTopicCreation(namespaceName, overridePolicy);
fail("Should have failed");
} catch (PulsarAdminException e) {
assertEquals(e.getStatusCode(), Status.PRECONDITION_FAILED.getStatusCode());
}
}

@Test
public void testMaxTopicsPerNamespace() throws Exception {
restartClusterAfterTest();
Expand Down
8 changes: 5 additions & 3 deletions .../main/java/org/apache/pulsar/common/policies/data/impl/AutoTopicCreationOverrideImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public static ValidateResult validateOverride(AutoTopicCreationOverride override
if (!TopicType.isValidTopicType(override.getTopicType())) {
return ValidateResult.fail(String.format("Unknown topic type [%s]", override.getTopicType()));
}

if (TopicType.PARTITIONED.toString().equals(override.getTopicType())) {
if (override.getDefaultNumPartitions() == null) {
return ValidateResult.fail("[defaultNumPartitions] cannot be null when the type is partitioned.");
Expand All @@ -52,9 +53,10 @@ public static ValidateResult validateOverride(AutoTopicCreationOverride override
return ValidateResult.fail("[defaultNumPartitions] cannot be less than 1 for partition type.");
}
} else if (TopicType.NON_PARTITIONED.toString().equals(override.getTopicType())) {
if (override.getDefaultNumPartitions() != null) {
return ValidateResult.fail("[defaultNumPartitions] is not allowed to be"
Comment thread
lhotari marked this conversation as resolved.
+ " set when the type is non-partition.");
Integer p = override.getDefaultNumPartitions();
if (p != null && p != 0 && p != 1) {
return ValidateResult.fail(
"[defaultNumPartitions] must be null, 0 or 1 when the type is non-partitioned.");
}
}
}
Expand Down
24 changes: 23 additions & 1 deletion ...n/src/test/java/org/apache/pulsar/common/policies/data/AutoTopicCreationOverrideTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,34 @@ public void testNumPartitionsNotSet() {
}

@Test
public void testNumPartitionsOnNonPartitioned() {
public void testNumPartitionsOnNonPartitionedZeroAllowed() {
AutoTopicCreationOverride override = AutoTopicCreationOverride.builder()
.allowAutoTopicCreation(true)
.topicType(TopicType.NON_PARTITIONED.toString())
.defaultNumPartitions(0)
.build();
assertTrue(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
}

@Test
public void testNumPartitionsOnNonPartitionedOneAllowed() {
AutoTopicCreationOverride override = AutoTopicCreationOverride.builder()
.allowAutoTopicCreation(true)
.topicType(TopicType.NON_PARTITIONED.toString())
.defaultNumPartitions(1)
.build();
assertTrue(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
}


@Test
public void testNumPartitionsOnNonPartitionedTooHighRejected() {
AutoTopicCreationOverride override = AutoTopicCreationOverride.builder()
.allowAutoTopicCreation(true)
.topicType(TopicType.NON_PARTITIONED.toString())
.defaultNumPartitions(2)
.build();
assertFalse(AutoTopicCreationOverrideImpl.validateOverride(override).isSuccess());
}

}