RobertIndie commented on code in PR #15010:
URL: https://github.com/apache/pulsar/pull/15010#discussion_r843744401
##########
tests/integration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java:
##########
@@ -113,16 +117,18 @@ private PulsarCluster(PulsarClusterSpec spec, CSContainer
csContainer, boolean s
prestoWorkerContainer = null;
}
-
this.zkContainer = new ZKContainer(clusterName);
+ this.metadataStoreContainer = createMetadataStoreContainer();
Review Comment:
Is that we still need to maintain the ZK container even though we use the
etcd container?
##########
tests/integration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java:
##########
@@ -133,10 +139,11 @@ private PulsarCluster(PulsarClusterSpec spec, CSContainer
csContainer, boolean s
this.proxyContainer = new
ProxyContainer(appendClusterName("pulsar-proxy"), ProxyContainer.NAME)
.withNetwork(network)
.withNetworkAliases(appendClusterName("pulsar-proxy"))
+ .withEnv("configurationStoreServers", CSContainer.NAME + ":" +
CS_PORT)
+ .withEnv("clusterName", clusterName)
.withEnv("zkServers", appendClusterName(ZKContainer.NAME))
.withEnv("zookeeperServers", appendClusterName(ZKContainer.NAME))
Review Comment:
Since we deprecated the `zookeeperServers`, I think we can remove it here.
The same is true for the broker.
##########
tests/integration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java:
##########
@@ -113,16 +117,18 @@ private PulsarCluster(PulsarClusterSpec spec, CSContainer
csContainer, boolean s
prestoWorkerContainer = null;
}
-
this.zkContainer = new ZKContainer(clusterName);
+ this.metadataStoreContainer = createMetadataStoreContainer();
+
this.zkContainer
.withNetwork(network)
.withNetworkAliases(appendClusterName(ZKContainer.NAME))
.withEnv("clusterName", clusterName)
.withEnv("zkServers", appendClusterName(ZKContainer.NAME))
.withEnv("configurationStore", CSContainer.NAME + ":" + CS_PORT)
.withEnv("forceSync", "no")
- .withEnv("pulsarNode", appendClusterName("pulsar-broker-0"));
+ .withEnv("pulsarNode", appendClusterName("pulsar-broker-0"))
Review Comment:
Can we wrap the zkConainer here as a metadataStoreContainer?
##########
tests/integration/src/test/java/org/apache/pulsar/tests/integration/topologies/PulsarCluster.java:
##########
@@ -146,18 +153,22 @@ private PulsarCluster(PulsarClusterSpec spec, CSContainer
csContainer, boolean s
// create bookies
bookieContainers.putAll(
- runNumContainers("bookie", spec.numBookies(), (name) -> new
BKContainer(clusterName, name)
- .withNetwork(network)
- .withNetworkAliases(appendClusterName(name))
- .withEnv("zkServers",
appendClusterName(ZKContainer.NAME))
- .withEnv("useHostNameAsBookieID", "true")
- // Disable fsyncs for tests since they're slow within
the containers
- .withEnv("journalSyncData", "false")
- .withEnv("journalMaxGroupWaitMSec", "0")
- .withEnv("clusterName", clusterName)
- .withEnv("diskUsageThreshold", "0.99")
- .withEnv("nettyMaxFrameSizeBytes", "" +
spec.maxMessageSize)
- )
+ runNumContainers("bookie", spec.numBookies(), (name) -> {
+ BKContainer bkContainer = new BKContainer(clusterName,
name)
Review Comment:
Why not return directly?
##########
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java:
##########
@@ -143,11 +143,11 @@ public ProxyServiceStarter(String[] args) throws
Exception {
// load config file
config = PulsarConfigurationLoader.create(configFile,
ProxyConfiguration.class);
- if (isBlank(metadataStoreUrl)) {
- // Use zookeeperServers from command line if metadataStoreUrl
is empty;
- config.setMetadataStoreUrl(zookeeperServers);
- } else {
- // Use metadataStoreUrl from command line
+ if (!isBlank(zookeeperServers)) {
+ config.setZookeeperServers(zookeeperServers);
Review Comment:
We have deprecated the `zookeeperServers` in the proxy configuration in
#13777. We would better not set it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
