[PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol #12056

EronWright · 2021-09-16T01:00:13Z

Master Issue: #12040

Motivation

Add a new configuration setting bindAddresses to open additional server ports on the broker. Note that these are in addition to brokerServicePort / brokerServicePortTls for compatibility reasons.

Each new-style bind address is associated with an advertised listener, to be used as the default listener for topic lookup requests. See #12040 for details. A given listener may be associated with numerous bindings.

The scheme indicates the protocol handler and whether to use TLS on the server channel. This PR is focused on the Pulsar protocol handler, but it is anticipated that other protocols may be supported in future.

For example:

bindAddresses=external:pulsar://0.0.0.0:6652,external:pulsar+ssl://0.0.0.0:6653
bindAddress=0.0.0.0
brokerServicePort=6650
brokerServicePortTls=6651
advertisedListeners=cluster:pulsar://broker-1.local:6650,cluster:pulsar+ssl://broker-1.local:6651,external:pulsar://broker-1.example.dev:6652,external:pulsar+ssl://broker-1.example.dev:6653
internalListenerName=cluster

The above would produce three server sockets, with 6650 having no associated listener name (thus retaining existing lookup behavior of returning the internal listener), and with 6652 and 6653 having an association with listener name external. Given a lookup request on 6652 or 6653, the external listener address would be returned.

Modifications

added configuration property bindAddresses
implementing parsing and validation logic
factored some utility code for formatting broker/web addresses

Verifying this change

Make sure that the change passes the CI checks.
Adhoc testing as outlined in PIP 95.

This change added tests and can be verified as follows:

Added unit tests for configuration validation logic.

Does this pull request potentially affect one of the following parts:

Anything that affects deployment: yes - new optional configuration properties

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

doc-required

…bind # Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

EronWright · 2021-09-17T16:47:12Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java


-        Optional<Integer> tlsPort = serviceConfig.getBrokerServicePortTls();
-        if (tlsPort.isPresent()) {
-            ServerBootstrap tlsBootstrap = bootstrap.clone();


An oddity in the old code is that bootstrap is cloned here, though it has the non-TLS child handler attached already. I assume the handler is replaced. I would expect the defaultServerBootstrap to be cloned, rather than taking a clone of a clone. See new code.

Anonymitaet · 2021-09-18T03:45:17Z

@EronWright thanks for your contribution!
One quick question: need to add the configuration and descriptions to Pulsar docs (https://pulsar.apache.org/docs/en/next/reference-configuration/)?

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

codelipenghui

Great work!

LGTM, just left a few comments.

conf/standalone.conf

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/validator/BindAddressValidator.java

EronWright · 2021-09-20T22:00:28Z

I've addressed the feedback, please take another look.

Huanli-Meng

A minor comment. PTAL. Thanks

site2/docs/reference-configuration.md

eolivelli

LGTM

We can merge to 2.9 if CI passes, I have restarted the failed jobs

cc @codelipenghui

codelipenghui · 2021-10-11T09:58:33Z

/pulsarbot run-failure-checks

BewareMyPower · 2021-10-11T13:08:23Z

/pulsarbot run-failure-checks

eolivelli · 2021-10-12T14:30:11Z

CI error is related to this patch

please fix

testMalformed(org.apache.pulsar.broker.validator.BindAddressValidatorTest)  Time elapsed: 0.033 s  <<< FAILURE!
java.lang.IllegalArgumentException: bindAddresses: malformed: internal:
	at org.apache.pulsar.broker.validator.BindAddressValidator.lambda$validateBindAddresses$0(BindAddressValidator.java:56)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234
```)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)

- incorporate feedback, fix test

EronWright · 2021-10-13T22:39:54Z

I believe this PR is ready to merge. The intermittent test failure seem unrelated.

Run 2: TopicPoliciesTest.testPolicyOverwrittenByNamespaceLevel:1034 » ConditionTimeout

* up/master: (26 commits) [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128) [website][upgrade]feat: docs migration - Development (apache#12320) Update delete inactive topic configuration documentation (apache#12350) [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056) Added Debezium Source for MS SQL Server (apache#12256) Fix: flaky oracle tests (apache#12306) [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341) [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335) feat(cli): add restart command to pulsar-daemon (apache#12279) [client-tools] Remove redundant initial value (apache#12296) Make AuthenticationTokenTest to run on windows (apache#12329) [offload] fix FileSystemManagedLedgerOffloader can not cleanup outdated ledger data (apache#12309) [Doc]--Update contents for Pulsar adaptor for Apache Spark (apache#12338) [PIP 95][Issue 12040][broker] Improved multi-listener in standalone mode (apache#12066) [website][upgrade]feat: docs migration - Cookbooks (apache#12319) [testclient] Make --payload-file take effect in PerformanceClient (apache#12187) [website][upgrade]feat: docs migration - adaptor (apache#12318) [pulsar-client] Add partition-change api for producer/consumer interceptors (apache#12287) [Transaction]Fix lowWaterMark of TopicTransactionBuffer (apache#12312) [pulsar-admin] New option takes precedence over deprecated option (apache#12260) ... # Conflicts: # site2/website-next/docusaurus.config.js # site2/website-next/versions.json

* up/master: (37 commits) re-enabling integration tests for Sinks (apache#12307) [PIP 95][Issue 12040][web] Topic lookup with listener header (apache#12072) Fix the master CI broken with update dispatch rate block issue (apache#12360) Fix message being ignored when the non-persistent topic reader reconnect. (apache#12348) Fix log format. (apache#12346) [website][upgrade]feat: docs migration - version-2.7.2 Concepts and Architecture (apache#12354) [website][upgrade] feat: full docs migration for version 2.8.0 (apache#12359) [website][upgrade]feat: dynamic replace version info before build (apache#12337) Fix flaky tests: ElasticSearchClientTests (apache#12347) Use asyncCloseCursorLedger to replace cursorLedger.asyncClose method in the ManagedCursorImpl.VoidCallback#operationComplete (apache#12113) fix-npe-ZkBookieRackAffinityMapping (apache#11947) [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128) [website][upgrade]feat: docs migration - Development (apache#12320) Update delete inactive topic configuration documentation (apache#12350) [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056) Added Debezium Source for MS SQL Server (apache#12256) Fix: flaky oracle tests (apache#12306) [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341) [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335) feat(cli): add restart command to pulsar-daemon (apache#12279) ... # Conflicts: # site2/website-next/docusaurus.config.js # site2/website-next/versioned_sidebars/version-2.7.2-sidebars.json # site2/website-next/versions.json

…ocol (apache#12056) Master Issue: apache#12040 ### Motivation Add a new configuration setting `bindAddresses` to open additional server ports on the broker. Note that these are in addition to `brokerServicePort` / `brokerServicePortTls` for compatibility reasons. Each new-style bind address is associated with an advertised listener, to be used as the default listener for topic lookup requests. See apache#12040 for details. A given listener may be associated with numerous bindings. The scheme indicates the protocol handler and whether to use TLS on the server channel. This PR is focused on the Pulsar protocol handler, but it is anticipated that other protocols may be supported in future. For example: ``` bindAddresses=external:pulsar://0.0.0.0:6652,external:pulsar+ssl://0.0.0.0:6653 bindAddress=0.0.0.0 brokerServicePort=6650 brokerServicePortTls=6651 advertisedListeners=cluster:pulsar://broker-1.local:6650,cluster:pulsar+ssl://broker-1.local:6651,external:pulsar://broker-1.example.dev:6652,external:pulsar+ssl://broker-1.example.dev:6653 internalListenerName=cluster ``` The above would produce three server sockets, with `6650` having no associated listener name (thus retaining existing lookup behavior of returning the internal listener), and with `6652` and `6653` having an association with listener name `external`. Given a lookup request on `6652` or `6653`, the `external` listener address would be returned. ### Modifications - added configuration property `bindAddresses` - implementing parsing and validation logic - factored some utility code for formatting broker/web addresses

EronWright added 6 commits September 15, 2021 13:34

[Issue 12040] Multiple bind addresses

48e9e29

[Issue 12040] Multiple bind addresses

1d5bedd

[Issue 12040] Multiple bind addresses

ced52a7

[Issue 12040] Multiple bind addresses

d3d18e6

[Issue 12040] Multiple bind addresses

0175fdb

Merge remote-tracking branch 'origin/master' into ewright/pip95-multi…

dcb749f

…bind # Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java

EronWright commented Sep 17, 2021

View reviewed changes

Anonymitaet added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Sep 18, 2021

BewareMyPower reviewed Sep 18, 2021

View reviewed changes

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java Outdated Show resolved Hide resolved

codelipenghui assigned EronWright Sep 18, 2021

codelipenghui added this to the 2.9.0 milestone Sep 18, 2021

codelipenghui reviewed Sep 18, 2021

View reviewed changes

codelipenghui requested review from 315157973, hangc0276, merlimat and sijie September 18, 2021 13:59

eolivelli changed the title ~~[Issue 12040][broker] Multiple bind addresses for Pulsar protocol~~ [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol Sep 20, 2021

[Issue 12040] Multiple bind addresses

f08f555

codelipenghui approved these changes Sep 22, 2021

View reviewed changes

Huanli-Meng reviewed Sep 22, 2021

View reviewed changes

site2/docs/reference-configuration.md Show resolved Hide resolved

eolivelli modified the milestones: 2.9.0, 2.10.0 Oct 6, 2021

eolivelli approved these changes Oct 8, 2021

View reviewed changes

codelipenghui modified the milestones: 2.10.0, 2.9.0 Oct 11, 2021

[Issue 12040][broker] Multiple bind addresses for Pulsar protocol

d696b83

- incorporate feedback, fix test

codelipenghui merged commit 3a337b8 into apache:master Oct 14, 2021

heesung-sn mentioned this pull request Jun 6, 2024

[fix][broker] Support advertised listeners when gracefully transferring topics (ExtensibleLoadManagerImpl only) #22862

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol #12056

[PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol #12056

EronWright commented Sep 16, 2021 •

edited

Loading

EronWright Sep 17, 2021

Anonymitaet commented Sep 18, 2021

codelipenghui left a comment

EronWright commented Sep 20, 2021

Huanli-Meng left a comment

eolivelli left a comment

codelipenghui commented Oct 11, 2021

BewareMyPower commented Oct 11, 2021

eolivelli commented Oct 12, 2021

EronWright commented Oct 13, 2021

[PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol #12056

[PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol #12056

Conversation

EronWright commented Sep 16, 2021 • edited Loading

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

EronWright Sep 17, 2021

Choose a reason for hiding this comment

Anonymitaet commented Sep 18, 2021

codelipenghui left a comment

Choose a reason for hiding this comment

EronWright commented Sep 20, 2021

Huanli-Meng left a comment

Choose a reason for hiding this comment

eolivelli left a comment

Choose a reason for hiding this comment

codelipenghui commented Oct 11, 2021

BewareMyPower commented Oct 11, 2021

eolivelli commented Oct 12, 2021

EronWright commented Oct 13, 2021

EronWright commented Sep 16, 2021 •

edited

Loading