Lazily init PulsarAdmin in PulsarAdminTool (apache#9312)

### Motivation pulsar-admin (PulsarAdminTool) initialises eagerly the PulsarAdmin object and some of the the underlying REST API intefaces. This initialisation process triggers lot of resource loading (like SSL/RESTAPI classes....) that slows down the JVM even for stuff that is not needed. Also there are shutdown hooks that are useless by they are executed while existing from the command. Removing initialisation of useless stuff helps in having a better bootstrap time, especially in case that you are not performing API calls, like when you are learning the tool and you make lots of syntax errors. ### Modifications - Initialise as lazily as possible PulsarAdmin - Make PulsarAdminTool#main "testable" by allowing it to not call System.exit - Use halt instead of exit in order to not trigger shutdown hooks ### Verifying this change This change is a trivial rework / code cleanup, but I have added tests for parts that have been touched and had not unit tests.
smiklosovic · Jan 28, 2021 · 232b324 · 232b324
1 parent 746f181
commit 232b324
Show file tree

Hide file tree

Showing 28 changed files with 654 additions and 492 deletions.
diff --git a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/CmdFunctionsTest.java b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/CmdFunctionsTest.java
@@ -117,9 +117,9 @@ public void setup() throws Exception {
         when(admin.functions()).thenReturn(functions);
         when(admin.getServiceUrl()).thenReturn("http://localhost:1234");
         when(admin.getClientConfigData()).thenReturn(new ClientConfigurationData());
-        this.cmd = new CmdFunctions(admin);
-        this.cmdSinks = new CmdSinks(admin);
-        this.cmdSources = new CmdSources(admin);
+        this.cmd = new CmdFunctions(() -> admin);
+        this.cmdSinks = new CmdSinks(() -> admin);
+        this.cmdSources = new CmdSources(() -> admin);
 
         // mock reflections
         mockStatic(Reflections.class);

diff --git a/...-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/DeprecatedCommanderTest.java b/...-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/DeprecatedCommanderTest.java
@@ -47,7 +47,7 @@ public void setup() {
         when(admin.topics()).thenReturn(mockTopics);
         mockSchemas = mock(Schemas.class);
         when(admin.schemas()).thenReturn(mockSchemas);
-        cmdTopics = new CmdTopics(admin);
+        cmdTopics = new CmdTopics(() -> admin);
     }
 
     @Test
@@ -71,7 +71,7 @@ public void testDeprecatedCommanderWorks() throws Exception {
         assertTrue(defaultOutput.contains("get-deduplication"));
 
         // annotation was changed to hidden, reset it.
-        cmdTopics = new CmdTopics(admin);
+        cmdTopics = new CmdTopics(() -> admin);
         CmdUsageFormatter formatter = (CmdUsageFormatter)cmdTopics.jcommander.getUsageFormatter();
         formatter.clearDeprecatedCommand();
         StringBuilder builder3 = new StringBuilder();

diff --git a/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java b/pulsar-client-tools-test/src/test/java/org/apache/pulsar/admin/cli/PulsarAdminToolTest.java
@@ -89,7 +89,7 @@ public void brokers() throws Exception {
         Brokers mockBrokers = mock(Brokers.class);
         doReturn(mockBrokers).when(admin).brokers();
 
-        CmdBrokers brokers = new CmdBrokers(admin);
+        CmdBrokers brokers = new CmdBrokers(() -> admin);
 
         brokers.run(split("list use"));
         verify(mockBrokers).getActiveBrokers("use");
@@ -125,7 +125,7 @@ public void brokerStats() throws Exception {
         BrokerStats mockBrokerStats = mock(BrokerStats.class);
         doReturn(mockBrokerStats).when(admin).brokerStats();
 
-        CmdBrokerStats brokerStats = new CmdBrokerStats(admin);
+        CmdBrokerStats brokerStats = new CmdBrokerStats(() -> admin);
 
         brokerStats.run(split("topics"));
         verify(mockBrokerStats).getTopics();
@@ -146,7 +146,7 @@ public void getOwnedNamespaces() throws Exception {
         Brokers mockBrokers = mock(Brokers.class);
         doReturn(mockBrokers).when(admin).brokers();
 
-        CmdBrokers brokers = new CmdBrokers(admin);
+        CmdBrokers brokers = new CmdBrokers(() -> admin);
 
         brokers.run(split("namespaces use --url http://my-service.url:4000"));
         verify(mockBrokers).getOwnedNamespaces("use", "http://my-service.url:4000");
@@ -159,7 +159,7 @@ public void clusters() throws Exception {
         Clusters mockClusters = mock(Clusters.class);
         when(admin.clusters()).thenReturn(mockClusters);
 
-        CmdClusters clusters = new CmdClusters(admin);
+        CmdClusters clusters = new CmdClusters(() -> admin);
 
         clusters.run(split("list"));
         verify(mockClusters).getClusters();
@@ -196,7 +196,7 @@ public void clusters() throws Exception {
 
         // Re-create CmdClusters to avoid a issue.
         // See https://github.com/cbeust/jcommander/issues/271
-        clusters = new CmdClusters(admin);
+        clusters = new CmdClusters(() -> admin);
 
         clusters.run(
                 split("create my-cluster --url http://my-service.url:8080 --url-secure https://my-service.url:4443"));
@@ -218,7 +218,7 @@ public void clusters() throws Exception {
         verify(mockClusters).getPeerClusterNames("my-cluster");
 
         // test create cluster without --url
-        clusters = new CmdClusters(admin);
+        clusters = new CmdClusters(() -> admin);
 
         clusters.run(split("create my-secure-cluster --url-secure https://my-service.url:4443"));
         verify(mockClusters).createCluster("my-secure-cluster", new ClusterData(null, "https://my-service.url:4443"));
@@ -236,7 +236,7 @@ public void tenants() throws Exception {
         Tenants mockTenants = mock(Tenants.class);
         when(admin.tenants()).thenReturn(mockTenants);
 
-        CmdTenants tenants = new CmdTenants(admin);
+        CmdTenants tenants = new CmdTenants(() -> admin);
 
         tenants.run(split("list"));
         verify(mockTenants).getTenants();
@@ -266,7 +266,7 @@ public void namespaces() throws Exception {
         Lookup mockLookup = mock(Lookup.class);
         when(admin.lookups()).thenReturn(mockLookup);
 
-        CmdNamespaces namespaces = new CmdNamespaces(admin);
+        CmdNamespaces namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("list myprop"));
         verify(mockNamespaces).getNamespaces("myprop");
@@ -319,7 +319,7 @@ public void namespaces() throws Exception {
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("unload myprop/clust/ns1 -b 0x80000000_0xffffffff"));
         verify(mockNamespaces).unloadNamespaceBundle("myprop/clust/ns1", "0x80000000_0xffffffff");
@@ -336,23 +336,23 @@ public void namespaces() throws Exception {
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("set-backlog-quota myprop/clust/ns1 -p producer_exception -l 10K"));
         verify(mockNamespaces).setBacklogQuota("myprop/clust/ns1",
                 new BacklogQuota(10 * 1024, RetentionPolicy.producer_exception));
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("set-backlog-quota myprop/clust/ns1 -p producer_exception -l 10M"));
         verify(mockNamespaces).setBacklogQuota("myprop/clust/ns1",
                 new BacklogQuota(10 * 1024 * 1024, RetentionPolicy.producer_exception));
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("set-backlog-quota myprop/clust/ns1 -p producer_exception -l 10G"));
         verify(mockNamespaces).setBacklogQuota("myprop/clust/ns1",
@@ -443,21 +443,21 @@ public void namespaces() throws Exception {
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("clear-backlog -b 0x80000000_0xffffffff myprop/clust/ns1 -force"));
         verify(mockNamespaces).clearNamespaceBundleBacklog("myprop/clust/ns1", "0x80000000_0xffffffff");
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("clear-backlog -s my-sub myprop/clust/ns1 -force"));
         verify(mockNamespaces).clearNamespaceBacklogForSubscription("myprop/clust/ns1", "my-sub");
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("clear-backlog -b 0x80000000_0xffffffff -s my-sub myprop/clust/ns1 -force"));
         verify(mockNamespaces).clearNamespaceBundleBacklogForSubscription("myprop/clust/ns1", "0x80000000_0xffffffff",
@@ -468,14 +468,14 @@ public void namespaces() throws Exception {
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("unsubscribe -b 0x80000000_0xffffffff -s my-sub myprop/clust/ns1"));
         verify(mockNamespaces).unsubscribeNamespaceBundle("myprop/clust/ns1", "0x80000000_0xffffffff", "my-sub");
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("get-max-producers-per-topic myprop/clust/ns1"));
         verify(mockNamespaces).getMaxProducersPerTopic("myprop/clust/ns1");
@@ -515,7 +515,7 @@ public void namespaces() throws Exception {
 
         mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        namespaces = new CmdNamespaces(admin);
+        namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("set-dispatch-rate myprop/clust/ns1 -md -1 -bd -1 -dt 2"));
         verify(mockNamespaces).setDispatchRate("myprop/clust/ns1", new DispatchRate(-1, -1, 2));
@@ -595,7 +595,7 @@ public void namespacesCreateV1() throws Exception {
         PulsarAdmin admin = Mockito.mock(PulsarAdmin.class);
         Namespaces mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        CmdNamespaces namespaces = new CmdNamespaces(admin);
+        CmdNamespaces namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("create my-prop/my-cluster/my-namespace"));
         verify(mockNamespaces).createNamespace("my-prop/my-cluster/my-namespace");
@@ -606,7 +606,7 @@ public void namespacesCreateV1WithBundlesAndClusters() throws Exception {
         PulsarAdmin admin = Mockito.mock(PulsarAdmin.class);
         Namespaces mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        CmdNamespaces namespaces = new CmdNamespaces(admin);
+        CmdNamespaces namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("create my-prop/my-cluster/my-namespace --bundles 5 --clusters a,b,c"));
         verify(mockNamespaces).createNamespace("my-prop/my-cluster/my-namespace", 5);
@@ -618,7 +618,7 @@ public void namespacesCreate() throws Exception {
         PulsarAdmin admin = Mockito.mock(PulsarAdmin.class);
         Namespaces mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        CmdNamespaces namespaces = new CmdNamespaces(admin);
+        CmdNamespaces namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("create my-prop/my-namespace"));
 
@@ -632,7 +632,7 @@ public void namespacesCreateWithBundlesAndClusters() throws Exception {
         PulsarAdmin admin = Mockito.mock(PulsarAdmin.class);
         Namespaces mockNamespaces = mock(Namespaces.class);
         when(admin.namespaces()).thenReturn(mockNamespaces);
-        CmdNamespaces namespaces = new CmdNamespaces(admin);
+        CmdNamespaces namespaces = new CmdNamespaces(() -> admin);
 
         namespaces.run(split("create my-prop/my-namespace --bundles 5 --clusters a,b,c"));
 
@@ -647,7 +647,7 @@ public void resourceQuotas() throws Exception {
         PulsarAdmin admin = Mockito.mock(PulsarAdmin.class);
         ResourceQuotas mockResourceQuotas = mock(ResourceQuotas.class);
         when(admin.resourceQuotas()).thenReturn(mockResourceQuotas);
-        CmdResourceQuotas cmdResourceQuotas = new CmdResourceQuotas(admin);
+        CmdResourceQuotas cmdResourceQuotas = new CmdResourceQuotas(() -> admin);
 
         ResourceQuota quota = new ResourceQuota();
         quota.setMsgRateIn(10);
@@ -666,7 +666,7 @@ public void resourceQuotas() throws Exception {
         // reset mocks
         mockResourceQuotas = mock(ResourceQuotas.class);
         when(admin.resourceQuotas()).thenReturn(mockResourceQuotas);
-        cmdResourceQuotas = new CmdResourceQuotas(admin);
+        cmdResourceQuotas = new CmdResourceQuotas(() -> admin);
 
         cmdResourceQuotas.run(split("get --namespace myprop/clust/ns1 --bundle 0x80000000_0xffffffff"));
         verify(mockResourceQuotas).getNamespaceBundleResourceQuota("myprop/clust/ns1", "0x80000000_0xffffffff");
@@ -686,7 +686,7 @@ public void namespaceIsolationPolicy() throws Exception {
         Clusters mockClusters = mock(Clusters.class);
         when(admin.clusters()).thenReturn(mockClusters);
 
-        CmdNamespaceIsolationPolicy nsIsolationPoliciesCmd = new CmdNamespaceIsolationPolicy(admin);
+        CmdNamespaceIsolationPolicy nsIsolationPoliciesCmd = new CmdNamespaceIsolationPolicy(() -> admin);
 
         nsIsolationPoliciesCmd.run(split("brokers use"));
         verify(mockClusters).getBrokersWithNamespaceIsolationPolicy("use");
@@ -703,7 +703,7 @@ public void topics() throws Exception {
         Schemas mockSchemas = mock(Schemas.class);
         when(admin.schemas()).thenReturn(mockSchemas);
 
-        CmdTopics cmdTopics = new CmdTopics(admin);
+        CmdTopics cmdTopics = new CmdTopics(() -> admin);
 
         cmdTopics.run(split("delete persistent://myprop/clust/ns1/ds1 -d"));
         verify(mockTopics).delete("persistent://myprop/clust/ns1/ds1", false);
@@ -912,7 +912,7 @@ public boolean matches(Long timestamp) {
         verify(mockTopics).removeMessageTTL("persistent://myprop/clust/ns1/ds1");
 
         //cmd with option cannot be executed repeatedly.
-        cmdTopics = new CmdTopics(admin);
+        cmdTopics = new CmdTopics(() -> admin);
         cmdTopics.run(split("reset-cursor persistent://myprop/clust/ns1/ds2 -s sub1 -m 1:1 -e"));
         verify(mockTopics).resetCursor(eq("persistent://myprop/clust/ns1/ds2"), eq("sub1")
                 , eq(new MessageIdImpl(1, 1, -1)), eq(true));
@@ -937,7 +937,7 @@ public void persistentTopics() throws Exception {
         Topics mockTopics = mock(Topics.class);
         when(admin.topics()).thenReturn(mockTopics);
 
-        CmdPersistentTopics topics = new CmdPersistentTopics(admin);
+        CmdPersistentTopics topics = new CmdPersistentTopics(() -> admin);
 
         topics.run(split("delete persistent://myprop/clust/ns1/ds1"));
         verify(mockTopics).delete("persistent://myprop/clust/ns1/ds1", false);
@@ -1022,7 +1022,7 @@ public void nonPersistentTopics() throws Exception {
         NonPersistentTopics mockTopics = mock(NonPersistentTopics.class);
         when(admin.nonPersistentTopics()).thenReturn(mockTopics);
 
-        CmdNonPersistentTopics topics = new CmdNonPersistentTopics(admin);
+        CmdNonPersistentTopics topics = new CmdNonPersistentTopics(() -> admin);
 
         topics.run(split("stats non-persistent://myprop/clust/ns1/ds1"));
         verify(mockTopics).getStats("non-persistent://myprop/clust/ns1/ds1");
@@ -1047,7 +1047,7 @@ public void bookies() throws Exception {
         Bookies mockBookies = mock(Bookies.class);
         doReturn(mockBookies).when(admin).bookies();
 
-        CmdBookies bookies = new CmdBookies(admin);
+        CmdBookies bookies = new CmdBookies(() -> admin);
 
         bookies.run(split("racks-placement"));
         verify(mockBookies).getBookiesRackInfo();
@@ -1142,7 +1142,7 @@ void proxy() throws Exception {
         ProxyStats mockProxyStats = mock(ProxyStats.class);
         doReturn(mockProxyStats).when(admin).proxyStats();
 
-        CmdProxyStats proxyStats = new CmdProxyStats(admin);
+        CmdProxyStats proxyStats = new CmdProxyStats(() -> admin);
 
         proxyStats.run(split("connections"));
         verify(mockProxyStats).getConnections();

diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBase.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBase.java
@@ -28,26 +28,36 @@
 import org.apache.pulsar.client.admin.PulsarAdminException;
 import org.apache.pulsar.client.admin.PulsarAdminException.ConnectException;
 
+import java.util.function.Supplier;
+
 public abstract class CmdBase {
     protected final JCommander jcommander;
-    protected final PulsarAdmin admin;
-    protected IUsageFormatter usageFormatter;
+    private final Supplier<PulsarAdmin> adminSupplier;
+    private PulsarAdmin admin;
+    private IUsageFormatter usageFormatter;
 
     @Parameter(names = { "-h", "--help" }, help = true, hidden = true)
     private boolean help;
 
-    public CmdBase(String cmdName, PulsarAdmin admin) {
-        this.admin = admin;
+    public CmdBase(String cmdName, Supplier<PulsarAdmin> adminSupplier) {
+        this.adminSupplier = adminSupplier;
         jcommander = new JCommander();
         usageFormatter = new CmdUsageFormatter(jcommander);
         jcommander.setProgramName("pulsar-admin " + cmdName);
         jcommander.setUsageFormatter(usageFormatter);
     }
 
+    protected IUsageFormatter getUsageFormatter() {
+        if (usageFormatter == null) {
+             usageFormatter = new DefaultUsageFormatter(jcommander);
+        }
+        return usageFormatter;
+    }
+
     private void tryShowCommandUsage() {
         try {
             String chosenCommand = jcommander.getParsedCommand();
-            usageFormatter.usage(chosenCommand);
+            getUsageFormatter().usage(chosenCommand);
         } catch (Exception e) {
             // it is caused by an invalid command, the invalid command can not be parsed
             System.err.println("Invalid command, please use `pulsar-admin --help` to check out how to use");
@@ -82,7 +92,7 @@ public boolean run(String[] args) {
             } catch (ConnectException e) {
                 System.err.println(e.getMessage());
                 System.err.println();
-                System.err.println("Error connecting to: " + admin.getServiceUrl());
+                System.err.println("Error connecting to: " + getAdmin().getServiceUrl());
                 return false;
             } catch (PulsarAdminException e) {
                 System.err.println(e.getHttpError());
@@ -95,4 +105,11 @@ public boolean run(String[] args) {
             }
         }
     }
+
+    protected PulsarAdmin getAdmin() {
+        if (admin == null) {
+            admin = adminSupplier.get();
+        }
+        return admin;
+    }
 }