You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/18 13:33:37 UTC
[GitHub] [pulsar] codelipenghui commented on a change in pull request #12056: [Issue 12040][broker] Multiple bind addresses for Pulsar protocol
codelipenghui commented on a change in pull request #12056:
URL: https://github.com/apache/pulsar/pull/12056#discussion_r711585850
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -121,13 +121,18 @@
private String configurationStoreServers;
@FieldContext(
category = CATEGORY_SERVER,
- doc = "The port for serving binary protobuf requests"
+ doc = "The port for serving binary protobuf requests."
+ + "If set, defines a server binding for bindAddress:brokerServicePort"
+ + "without an associated advertised listener."
+ + "The Default value is 6650."
)
private Optional<Integer> brokerServicePort = Optional.of(6650);
@FieldContext(
category = CATEGORY_SERVER,
- doc = "The port for serving tls secured binary protobuf requests"
+ doc = "The port for serving TLS-secured binary protobuf requests."
+ + "If set, defines a server binding for bindAddress:brokerServicePortTls"
+ + "without an associated advertised listener."
Review comment:
Same as the above comment.
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/validator/BindAddressValidator.java
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.validator;
+
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.ArrayList;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.ServiceConfigurationUtils;
+import org.apache.pulsar.common.configuration.BindAddress;
+
+import static java.util.stream.Collectors.toList;
+
+/**
+ * Validates bind address configurations.
+ */
+public class BindAddressValidator {
+
+ private static final Pattern bindAddress = Pattern.compile("(?<name>\\w+):(?<url>.+)$");
Review comment:
```suggestion
private static final Pattern BIND_ADDRESSES_PATTERN = Pattern.compile("(?<name>\\w+):(?<url>.+)$");
```
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/validator/BindAddressValidator.java
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.validator;
+
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.ArrayList;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.ServiceConfigurationUtils;
+import org.apache.pulsar.common.configuration.BindAddress;
+
+import static java.util.stream.Collectors.toList;
+
+/**
+ * Validates bind address configurations.
+ */
+public class BindAddressValidator {
+
+ private static final Pattern bindAddress = Pattern.compile("(?<name>\\w+):(?<url>.+)$");
+
+ /**
+ * Validate the configuration of `bindAddresses`.
+ * @param config the pulsar broker configure.
+ * @param schemes a filter on the schemes of the bind addresses, or null to not apply a filter.
+ * @return a list of bind addresses.
+ */
+ public static List<BindAddress> validateBindAddresses(ServiceConfiguration config, Collection<String> schemes) {
+ // migrate the existing configuration properties
+ List<BindAddress> addresses = migrateBindAddresses(config);
+
+ // parse the list of additional bind addresses
+ Arrays
+ .stream(StringUtils.split(StringUtils.defaultString(config.getBindAddresses()), ","))
+ .map(bindAddress::matcher)
+ .filter(Matcher::matches)
Review comment:
If users provided an invalid `bind address`, we should make a clear prompt to users and stop to start the broker.
For example, if `pulsar://xxxxx:6650` is used, we will use `pulsar` as the listener name.
or `xxx:yyy:pulsar://xxxxx:6650`, we will use `xxx` as the listener name.
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/validator/BindAddressValidator.java
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.validator;
+
+import java.net.URI;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.ArrayList;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.broker.ServiceConfigurationUtils;
+import org.apache.pulsar.common.configuration.BindAddress;
+
+import static java.util.stream.Collectors.toList;
+
+/**
+ * Validates bind address configurations.
+ */
+public class BindAddressValidator {
Review comment:
It is better to add a check to avoid using the same bind port in the bindAddresses.
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -121,13 +121,18 @@
private String configurationStoreServers;
@FieldContext(
category = CATEGORY_SERVER,
- doc = "The port for serving binary protobuf requests"
+ doc = "The port for serving binary protobuf requests."
+ + "If set, defines a server binding for bindAddress:brokerServicePort"
+ + "without an associated advertised listener."
Review comment:
The default port can also associate with one or more advertised listeners? The current behavior is users can create advertised listeners for one bind port.
##########
File path: conf/standalone.conf
##########
@@ -33,6 +33,9 @@ webServicePort=8080
# Hostname or IP address the service binds on, default is 0.0.0.0.
bindAddress=0.0.0.0
+# Extra bind addresses for protocol handlers: <listener_name>:<scheme>://<host>:<port>,[...]
+bindAddresses=
Review comment:
I think it is not only for the protocol handlers? Without the protocol handlers, Pulsar still can bind different addresses for the internal/external access requirement. Maybe describe as `Used to bind multiple addresses for the broker` is reasonable here?
And we should also apply the change in the broker.conf
Is it better to avoid using `bindAddresses` and `bindAddress` together?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org