[fix][broker] Fix Npe thrown by splitBundle (apache#17370)

yangl · Sep 1, 2022 · 1668c07 · 1668c07
1 parent 9529850
commit 1668c07
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
@@ -1158,6 +1158,10 @@ protected CompletableFuture<Void> internalSplitNamespaceBundleAsync(String bundl
                 .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
                 .thenCompose(policies->{
                     String bundleRange = getBundleRange(bundleName);
+                    if (bundleRange == null) {
+                        throw new RestException(Status.NOT_FOUND,
+                                String.format("Bundle range %s not found", bundleName));
+                    }
                     return validateNamespaceBundleOwnershipAsync(namespaceName, policies.bundles, bundleRange,
                             authoritative, false)
                             .thenCompose(nsBundle -> pulsar().getNamespaceService().splitAndOwnBundle(nsBundle, unload,
@@ -1216,13 +1220,18 @@ protected CompletableFuture<TopicHashPositions> internalGetTopicHashPositionsAsy
     }
 
     private String getBundleRange(String bundleName) {
+        NamespaceBundle nsBundle;
         if (BundleType.LARGEST.toString().equals(bundleName)) {
-            return findLargestBundleWithTopics(namespaceName).getBundleRange();
+            nsBundle = findLargestBundleWithTopics(namespaceName);
         } else if (BundleType.HOT.toString().equals(bundleName)) {
-            return findHotBundle(namespaceName).getBundleRange();
+            nsBundle = findHotBundle(namespaceName);
         } else {
             return bundleName;
         }
+        if (nsBundle == null) {
+            return null;
+        }
+        return nsBundle.getBundleRange();
     }
 
     private NamespaceBundle findLargestBundleWithTopics(NamespaceName namespaceName) {

diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
@@ -59,6 +59,7 @@
 import org.apache.pulsar.broker.service.BrokerTestBase;
 import org.apache.pulsar.broker.service.Topic;
 import org.apache.pulsar.broker.service.persistent.PersistentTopic;
+import org.apache.pulsar.client.admin.PulsarAdminException;
 import org.apache.pulsar.client.api.Consumer;
 import org.apache.pulsar.client.api.PulsarClient;
 
@@ -595,6 +596,26 @@ public void testSplitLargestBundle() throws Exception {
         }
     }
 
+    public void testSplitBUndleWithNoBundle() throws  Exception {
+        conf.setLoadManagerClassName(ModularLoadManagerImpl.class.getName());
+        restartBroker();
+        String namespace = "prop/test/ns-abc2";
+
+        BundlesData bundleData = BundlesData.builder().numBundles(10).build();
+        admin.namespaces().createNamespace(namespace, bundleData);
+
+        NamespaceService namespaceService = pulsar.getNamespaceService();
+        NamespaceName nsname = NamespaceName.get(namespace);
+        NamespaceBundles bundles = namespaceService.getNamespaceBundleFactory().getBundles(nsname);
+
+        try {
+            admin.namespaces().splitNamespaceBundle(namespace, Policies.BundleType.HOT.toString(), false, null);
+            fail("should have failed.");
+        } catch (Exception ex) {
+            Assert.assertEquals(404, ((PulsarAdminException) ex).getStatusCode());
+            Assert.assertEquals("Bundle range HOT not found", ex.getMessage());
+        }
+    }
     /**
      * Test bundle split with hot bundle which is serving highest load.
      *