Issue 6478: Fix the misleading setting in presto configuration (apach…

…e#8549) *Motivation* Fixes apache#6748 This is a fork of apache#6757 after rebased to the latest master.
lujp · May 22, 2021 · f807fed · f807fed
1 parent bad185a
commit f807fed
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 9 deletions.
diff --git a/conf/presto/catalog/pulsar.properties b/conf/presto/catalog/pulsar.properties
@@ -20,7 +20,10 @@
 # name of the connector to be displayed in the catalog
 connector.name=pulsar
 # the url of Pulsar broker service
-pulsar.broker-service-url=http://localhost:8080
+# DEPRECATED
+pulsar.broker-service-url=http://localhost:8080 
+# the url of Pulsar broker web service
+pulsar.web-service-url=http://localhost:8080
 # URI of Zookeeper cluster
 pulsar.zookeeper-uri=127.0.0.1:2181
 # minimum number of entries to read at a single time

diff --git a/...r-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarConnectorConfig.java b/...r-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarConnectorConfig.java
@@ -28,6 +28,7 @@
 import javax.validation.constraints.NotNull;
 import javax.ws.rs.client.ClientBuilder;
 import org.apache.bookkeeper.stats.NullStatsProvider;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.pulsar.client.admin.PulsarAdmin;
 import org.apache.pulsar.client.admin.PulsarAdminBuilder;
 import org.apache.pulsar.client.api.PulsarClientException;
@@ -42,6 +43,7 @@
 public class PulsarConnectorConfig implements AutoCloseable {
 
     private String brokerServiceUrl = "http://localhost:8080";
+    private String webServiceUrl = ""; //leave empty
     private String zookeeperUri = "localhost:2181";
     private int entryReadBatchSize = 100;
     private int targetNumSplits = 2;
@@ -86,14 +88,26 @@ public class PulsarConnectorConfig implements AutoCloseable {
 
     @NotNull
     public String getBrokerServiceUrl() {
-        return brokerServiceUrl;
+        if (StringUtils.isEmpty(webServiceUrl)){
+            return brokerServiceUrl;
+        } else {
+            return getWebServiceUrl();
+        }
     }
-
     @Config("pulsar.broker-service-url")
     public PulsarConnectorConfig setBrokerServiceUrl(String brokerServiceUrl) {
         this.brokerServiceUrl = brokerServiceUrl;
         return this;
     }
+    @Config("pulsar.web-service-url")
+    public PulsarConnectorConfig setWebServiceUrl(String webServiceUrl) {
+        this.webServiceUrl = webServiceUrl;
+        return this;
+    }
+
+    public String getWebServiceUrl() {
+        return webServiceUrl;
+    }
 
     @Config("pulsar.max-message-size")
     public PulsarConnectorConfig setMaxMessageSize(int maxMessageSize) {
@@ -454,8 +468,14 @@ public void close() throws Exception {
 
     @Override
     public String toString() {
-        return "PulsarConnectorConfig{"
+       if (StringUtils.isEmpty(webServiceUrl)){
+           return "PulsarConnectorConfig{"
             + "brokerServiceUrl='" + brokerServiceUrl + '\''
             + '}';
+        } else {
+            return "PulsarConnectorConfig{"
+            + "brokerServiceUrl='" + webServiceUrl + '\''
+            + '}';
+        }
     }
 }
diff --git a/site2/docs/sql-deployment-configurations.md b/site2/docs/sql-deployment-configurations.md
@@ -14,7 +14,7 @@ You can configure Presto Pulsar Connector in the `${project.root}/conf/presto/ca
 connector.name=pulsar
 
 # the url of Pulsar broker service
-pulsar.broker-service-url=http://localhost:8080
+pulsar.web-service-url=http://localhost:8080
 
 # URI of Zookeeper cluster
 pulsar.zookeeper-uri=localhost:2181
@@ -26,10 +26,10 @@ pulsar.entry-read-batch-size=100
 pulsar.target-num-splits=4
 ```
 
-You can connect Presto to a Pulsar cluster with multiple hosts. To configure multiple hosts for brokers, add multiple URLs to `pulsar.broker-service-url`. To configure multiple hosts for ZooKeeper, add multiple URIs to `pulsar.zookeeper-uri`. The following is an example.
+You can connect Presto to a Pulsar cluster with multiple hosts. To configure multiple hosts for brokers, add multiple URLs to `pulsar.web-service-url`. To configure multiple hosts for ZooKeeper, add multiple URIs to `pulsar.zookeeper-uri`. The following is an example.
 
 ```
-pulsar.broker-service-url=http://localhost:8080,localhost:8081,localhost:8082
+pulsar.web-service-url=http://localhost:8080,localhost:8081,localhost:8082
 pulsar.zookeeper-uri=localhost1,localhost2:2181
 ```
 
@@ -121,7 +121,7 @@ query.max-memory-per-node=1GB
 discovery.uri=<coordinator-url>
 ```
 
-2. Modify `pulsar.broker-service-url` and  `pulsar.zookeeper-uri` configuration in the `${project.root}/conf/presto/catalog/pulsar.properties` file accordingly for the three nodes.
+2. Modify `pulsar.web-service-url` and  `pulsar.zookeeper-uri` configuration in the `${project.root}/conf/presto/catalog/pulsar.properties` file accordingly for the three nodes.
 
 3. Start the coordinator node.
 

diff --git a/...tegration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java b/...tegration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java
@@ -418,7 +418,7 @@ private PrestoWorkerContainer buildPrestoWorkerContainer(String hostName, boolea
                 .withEnv("zkServers", ZKContainer.NAME)
                 .withEnv("zookeeperServers", ZKContainer.NAME + ":" + ZKContainer.ZK_PORT)
                 .withEnv("pulsar.zookeeper-uri", ZKContainer.NAME + ":" + ZKContainer.ZK_PORT)
-                .withEnv("pulsar.broker-service-url", "http://pulsar-broker-0:8080")
+                .withEnv("pulsar.web-service-url", "http://pulsar-broker-0:8080")
                 .withClasspathResourceMapping(
                         resourcePath, "/pulsar/conf/presto/config.properties", BindMode.READ_WRITE);
         if (spec.queryLastMessage) {