[fix][broker] Fix lookup heartbeat and sla namespace bundle when using extensible load manager #21213

Demogorgon314 · 2023-09-21T02:27:23Z

Motivation

Currently, if the cluster has multiple brokers, and the cluster is doing rolling restart, the heartbeat namespace topic's lookup result might be wrong, because the ExtensibleLoadManagerImpl does not check the heartbeat and SLA namespace bundle lookup candidate broker to let them own by the specified broker.

This ownership selection is wrong:

2023-09-20T08:27:32,188+0000 [ForkJoinPool.commonPool-worker-1] INFO  org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImpl - Selected new owner broker: broker-2:8080 for bundle: pulsar/broker-0:8080/0x00000000_0xffffffff.

After the ownership assignment, the broker-0 will fail to start.

pulsar-broker 2023-09-20T08:27:54,707+0000 [main] INFO  org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImpl - Try acquiring ownership for bundle: pulsar/broker-0:8080/0x00000000_0xffffffff - broker-0:8080.
pulsar-broker 2023-09-20T08:27:54,707+0000 [main] ERROR org.apache.pulsar.broker.namespace.NamespaceService - namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerNamespace(NamespaceService.java:400) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerBootstrapNamespaces(NamespaceService.java:343) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:863) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter$BrokerStarter.start(PulsarBrokerStarter.java:276) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter.main(PulsarBrokerStarter.java:356) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker 2023-09-20T08:27:54,710+0000 [main] ERROR org.apache.pulsar.broker.PulsarService - Failed to start Pulsar service: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker org.apache.pulsar.broker.PulsarServerException: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerNamespace(NamespaceService.java:403) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerBootstrapNamespaces(NamespaceService.java:343) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:863) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter$BrokerStarter.start(PulsarBrokerStarter.java:276) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter.main(PulsarBrokerStarter.java:356) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker Caused by: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerNamespace(NamespaceService.java:400) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     ... 4 more
pulsar-broker 2023-09-20T08:27:54,710+0000 [main] ERROR org.apache.pulsar.PulsarBrokerStarter - Failed to start pulsar service.
pulsar-broker org.apache.pulsar.broker.PulsarServerException: org.apache.pulsar.broker.PulsarServerException: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:938) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter$BrokerStarter.start(PulsarBrokerStarter.java:276) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.PulsarBrokerStarter.main(PulsarBrokerStarter.java:356) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker Caused by: org.apache.pulsar.broker.PulsarServerException: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerNamespace(NamespaceService.java:403) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerBootstrapNamespaces(NamespaceService.java:343) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:863) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     ... 2 more
pulsar-broker Caused by: java.lang.IllegalStateException: namespace already owned by other broker : ns=pulsar/broker-0:8080 expected=pulsar://broker-0:6650 actual=pulsar://broker-2:6650
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerNamespace(NamespaceService.java:400) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.namespace.NamespaceService.registerBootstrapNamespaces(NamespaceService.java:343) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     at org.apache.pulsar.broker.PulsarService.start(PulsarService.java:863) ~[io.streamnative-pulsar-broker-3.1.0-SNAPSHOT.jar:3.1.0-SNAPSHOT]
pulsar-broker     ... 2 more
pulsar-broker 2023-09-20T08:27:54,711+0000 [main] WARN  org.apache.pulsar.common.util.ShutdownUtil - Triggering immediate shutdown of current process with status 1

Modifications

Fix lookup heartbeat and sla namespace bundle when using extensible load manager
When updating the topK bundles, filter out the heartbeat and SLA namespace bundles.
Skip override orphan heartbeat namespace bundle

Documentation

doc
doc-required
doc-not-needed
doc-complete

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java

Demogorgon314 · 2023-09-22T03:48:50Z

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

+            this.isolationPoliciesHelper = new IsolationPoliciesHelper(policies);
+            this.brokerFilterPipeline.add(new BrokerIsolationPoliciesFilter(isolationPoliciesHelper));
+
+            createSystemTopic(pulsar, BROKER_LOAD_DATA_STORE_TOPIC);


@heesung-sn Somehow, the creation topic might time out, so the broker will shut down. And the brokerRegistry is not closed, the zk path exists, and the broker start will fail again.

BTW, we might need to add a retry for system topic creation, and ignore the topic already exists exception.

We do skip if it already exists. If the system topic creation fails here, k8s should restart the broker.

Do you know why the topic creation will time out? I saw several times, but after broker restarted, it will be successful start.

I am not sure. Do we have some logs/strack-traces?

...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java

heesung-sn · 2023-09-26T16:18:37Z

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

+            this.isolationPoliciesHelper = new IsolationPoliciesHelper(policies);
+            this.brokerFilterPipeline.add(new BrokerIsolationPoliciesFilter(isolationPoliciesHelper));
+
+            createSystemTopic(pulsar, BROKER_LOAD_DATA_STORE_TOPIC);


We do skip if it already exists. If the system topic creation fails here, k8s should restart the broker.

heesung-sn · 2023-09-26T16:19:29Z

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

@@ -207,6 +210,36 @@ public Set<NamespaceBundle> getOwnedServiceUnits() {
                    var bundle = entry.getKey();
                    return getNamespaceBundle(pulsar, bundle);
                }).collect(Collectors.toSet());
+        // Add heartbeat and SLA monitor namespace bundle.
+        NamespaceName heartbeatNamespace = NamespaceService.getHeartbeatNamespace(brokerId, pulsar.getConfiguration());


Why do we need to add these static bundles(bundles that are not stored in BSC) for getOwnedServiceUnits?

Yes, for getOwnedServiceUnits it needs to know the heartbeat and SLA monitor the ownership. it just keeps the behavior the same as previously.

heesung-sn · 2023-09-26T16:23:27Z

...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java

-                    orphanServiceUnits.put(serviceUnit, stateData);
-                }
-            } else if (now - stateData.timestamp() > semiTerminalStateWaitingTimeInMillis) {
+            if (isActiveState(state) && StringUtils.isNotBlank(srcBroker) && !activeBrokers.contains(srcBroker)) {


nit: isActiveState(state) check is repeated in the below cases.

I'm more like to return first, so the logic can be easy to understand. Of course, I can revert this change since not relevant, what do you think?

im fine with this change.

pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java

...src/main/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerImpl.java

…oad manager

codecov-commenter · 2023-10-07T15:47:40Z

Codecov Report

Merging #21213 (a812637) into master (643428b) will increase coverage by 36.27%.
Report is 3 commits behind head on master.
The diff coverage is 80.00%.

@@              Coverage Diff              @@
##             master   #21213       +/-   ##
=============================================
+ Coverage     36.96%   73.23%   +36.27%     
- Complexity    12294    32505    +20211     
=============================================
  Files          1698     1887      +189     
  Lines        130510   140223     +9713     
  Branches      14260    15435     +1175     
=============================================
+ Hits          48240   102694    +54454     
+ Misses        75982    29435    -46547     
- Partials       6288     8094     +1806

Flag	Coverage Δ
inttests	`24.17% <6.11%> (-0.16%)`	⬇️
systests	`24.69% <5.00%> (-0.20%)`	⬇️
unittests	`72.51% <80.00%> (+40.39%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...org/apache/bookkeeper/mledger/util/RangeCache.java	`93.10% <100.00%> (+19.23%)`	⬆️
...n/java/org/apache/pulsar/broker/PulsarService.java	`82.69% <100.00%> (+13.80%)`	⬆️
...ker/loadbalance/extensions/models/TopKBundles.java	`90.78% <100.00%> (+90.78%)`	⬆️
...org/apache/pulsar/broker/loadbalance/LoadData.java	`91.66% <0.00%> (+25.00%)`	⬆️
...ache/pulsar/broker/namespace/NamespaceService.java	`71.94% <75.00%> (+28.17%)`	⬆️
...xtensions/channel/ServiceUnitStateChannelImpl.java	`84.48% <58.82%> (+83.94%)`	⬆️
...dbalance/extensions/ExtensibleLoadManagerImpl.java	`77.75% <82.39%> (+75.66%)`	⬆️

... and 1446 files with indirect coverage changes

…g extensible load manager (apache#21213) (cherry picked from commit f85e0dc)

…g extensible load manager (apache#21213)

…le when using extensible load manager (#21213) (#21314)

Technoboy- · 2023-10-19T13:55:18Z

Cherry-pick by #21314

…g extensible load manager (#21213) (#21313)

heesung-sn · 2023-12-13T17:47:07Z

...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java

+                continue;
+            }
+
+            if (now - stateData.timestamp() > semiTerminalStateWaitingTimeInMillis) {


Sorry. I missed this.
This should be if(!isActiveState(state) && now - stateData.timestamp() > semiTerminalStateWaitingTimeInMillis) . Otherwise, we will clean active states, including Owned.

…le when using extensible load manager (apache#21213) (apache#21314)

…le when using extensible load manager (apache#21213) (apache#21314) (cherry picked from commit 0454410)

Demogorgon314 added type/bug The PR fixed a bug or issue reported a bug area/broker release/3.0.2 release/3.1.1 labels Sep 21, 2023

Demogorgon314 self-assigned this Sep 21, 2023

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 21, 2023

Demogorgon314 added the ready-to-test label Sep 21, 2023

Demogorgon314 requested review from heesung-sn, Technoboy-, codelipenghui and gaoran10 September 21, 2023 05:49

heesung-sn reviewed Sep 21, 2023

View reviewed changes

Demogorgon314 commented Sep 22, 2023

View reviewed changes

heesung-sn reviewed Sep 25, 2023

View reviewed changes

...ava/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java Outdated Show resolved Hide resolved

Technoboy- added this to the 3.2.0 milestone Sep 26, 2023

Technoboy- approved these changes Sep 26, 2023

View reviewed changes

heesung-sn approved these changes Sep 26, 2023

View reviewed changes

gaoran10 reviewed Sep 27, 2023

View reviewed changes

gaoran10 approved these changes Sep 28, 2023

View reviewed changes

Demogorgon314 force-pushed the Demogorgon314/fix-lookup-heartbeat-and-sla-namespace-bundle branch from 6586a05 to decf7fa Compare September 30, 2023 02:22

Demogorgon314 added 10 commits October 7, 2023 16:50

Fix lookup heartbeat and sla namespace bundle when using extensible l…

d693909

…oad manager

Filter out the extensible load manager brokers from modular load manager

0b71fef

Skip override orphan heartbeat namespace bundle

72080c6

fix test

4d5754c

Do not publish heartbeat bundle owner message to service unit

65cb374

Make broker registry start after serviceUnitStateChannel

2aff5c7

revert filter available broker by load manager class

a18df9a

Fix test

7986490

Optimize dedupeLookupRequest

e4920b1

Update the error log

031661a

Demogorgon314 added 5 commits October 7, 2023 16:50

refactor test

9b3ebbf

refactor test

cd0429e

Clean old logic

4ed0a14

Fix code style

2b658a7

Use ConcurrentHashMap

a812637

Demogorgon314 force-pushed the Demogorgon314/fix-lookup-heartbeat-and-sla-namespace-bundle branch from decf7fa to a812637 Compare October 7, 2023 08:50

Demogorgon314 merged commit f85e0dc into apache:master Oct 8, 2023
45 checks passed

Demogorgon314 deleted the Demogorgon314/fix-lookup-heartbeat-and-sla-namespace-bundle branch October 8, 2023 00:52

Demogorgon314 added a commit to Demogorgon314/pulsar that referenced this pull request Oct 8, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when usin…

31fc736

…g extensible load manager (apache#21213) (cherry picked from commit f85e0dc)

poorbarcode added the cherry-picked/branch-3.0 label Oct 8, 2023

Demogorgon314 added a commit to Demogorgon314/pulsar that referenced this pull request Oct 8, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when usin…

f242022

…g extensible load manager (apache#21213) (cherry picked from commit f85e0dc)

liangyuanpeng pushed a commit to liangyuanpeng/pulsar that referenced this pull request Oct 11, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when usin…

30c9002

…g extensible load manager (apache#21213)

vinayakmalik95 pushed a commit to tmdc-io/pulsar that referenced this pull request Oct 12, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when usin…

64cec9e

…g extensible load manager (apache#21213)

Demogorgon314 added a commit that referenced this pull request Oct 18, 2023

[fix][broker][branch-3.1] Fix lookup heartbeat and sla namespace bund…

0454410

…le when using extensible load manager (#21213) (#21314)

Technoboy- added cherry-picked/branch-3.1 release/3.1.2 and removed release/3.1.1 labels Oct 19, 2023

Technoboy- pushed a commit that referenced this pull request Oct 19, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when usin…

f26589e

…g extensible load manager (#21213) (#21313)

heesung-sn reviewed Dec 13, 2023

View reviewed changes

nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023

[fix][broker][branch-3.1] Fix lookup heartbeat and sla namespace bund…

5f358ec

…le when using extensible load manager (apache#21213) (apache#21314)

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 20, 2023

[fix][broker][branch-3.1] Fix lookup heartbeat and sla namespace bund…

f027857

…le when using extensible load manager (apache#21213) (apache#21314)

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Feb 29, 2024

[fix][broker][branch-3.1] Fix lookup heartbeat and sla namespace bund…

0e6c049

…le when using extensible load manager (apache#21213) (apache#21314) (cherry picked from commit 0454410)

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 6, 2024

[fix][broker][branch-3.1] Fix lookup heartbeat and sla namespace bund…

3e1825d

…le when using extensible load manager (apache#21213) (apache#21314) (cherry picked from commit 0454410)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix][broker] Fix lookup heartbeat and sla namespace bundle when using extensible load manager #21213

[fix][broker] Fix lookup heartbeat and sla namespace bundle when using extensible load manager #21213

Demogorgon314 commented Sep 21, 2023 •

edited

Demogorgon314 Sep 22, 2023 •

edited

Demogorgon314 Sep 22, 2023

heesung-sn Sep 26, 2023

Demogorgon314 Sep 27, 2023

heesung-sn Sep 27, 2023

heesung-sn Sep 26, 2023

heesung-sn Sep 26, 2023

Demogorgon314 Sep 27, 2023

heesung-sn Sep 26, 2023

Demogorgon314 Sep 27, 2023

heesung-sn Sep 27, 2023

codecov-commenter commented Oct 7, 2023

Technoboy- commented Oct 19, 2023

heesung-sn Dec 13, 2023

[fix][broker] Fix lookup heartbeat and sla namespace bundle when using extensible load manager #21213

[fix][broker] Fix lookup heartbeat and sla namespace bundle when using extensible load manager #21213

Conversation

Demogorgon314 commented Sep 21, 2023 • edited

Motivation

Modifications

Documentation

Demogorgon314 Sep 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 7, 2023

Codecov Report

Technoboy- commented Oct 19, 2023

Choose a reason for hiding this comment

Demogorgon314 commented Sep 21, 2023 •

edited

Demogorgon314 Sep 22, 2023 •

edited