[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946

poorbarcode · 2024-01-22T20:09:29Z

Motivation

There is a race condition that makes an orphan replicator in the original owner of a topic, and causes the new owner of the topic can not start a replicator due to org.apache.pulsar.broker.service.BrokerServiceException$NamingException Producer with name 'pulsar.repl.{local_cluster}-->{remote_cluster}' is already connected to topic.

Scenario 1

Thread-1: start/restart the producer of the replicator.
Thread-2: unloading bundles.

Scenario 2

Thread-1: start a new replicator after updated replication_clusters.
Thread-2: unloading bundles.

Current PR is focusing on Scenario 1.

Steps of Scenario 1

time	`thread start replicator`	thread `unload bundle`
1	Initialize cursor `pulsar.repl`
2	Start producer
3	Start producer failed, add a scheduled task to retry
4		Mark topic as `closing`
4		Close clients: `replicator.disconnect`
5		Skip to close the producer because the producer is null, and set `replicator.stat --> Stopped`
6	Retry to start the producer
7	Set `replicator.stat --> Starting`
8	Create producer success and set `replicator.stat --> Started`
9	Trigger a `readMoreEntries`, since there is no entries to read, just pending this request
10		Close cursor `pulsar.repl`
11		Close managed ledger
12	An orphan replicator is there, and the next topic owner could not start a replicator due to `Producer with name 'pulsar.repl.{local_cluster}-->{remote_cluster}' is already connected to topic`

Modifications

Split the state of Replicator.State.Stopped into Producer_Stopped and Closed.
- The producer can be restart again after the producer closed due to read entries error or ack messages error.
- The Replicator can not be started again after it was closed due to the topic being closed or having disabled replication.
Add a new method terminate to close the Replicator.
- The old method disconnect only used to close the internal producer.

A case that hit this issue

2024-01-19T22:18:24,498+0000 [pulsar-io-4-23] ERROR org.apache.pulsar.client.impl.ProducerImpl - [persistent://xxx_stress.Default-226] [pulsar.repl.c1-->c2] Failed to create producer: {"errorMsg":"org.apache.pulsar.broker.service.BrokerServiceException$NamingException: Producer with name 'pulsar.repl.c1-->c2' is already connected to topic","reqId":885581189144276683, "remote":"xxx/xxx:6651", "local":"/xxx:59416"}	
...

Picture-1: An orphan producer was left in old broker, it is not associated with any topic/replicator

Picture-2: After the topic is transferred to new broker, it can not start a new Replicator successfully

Since the scenario is too complex, I can not add a test. But I reproduced the Scenario 1 locally.

#21948 fixes the following issues:

How to perfectly start a new Replicator after the replication has been enabled again.
How to perfectly start a new Replicator when calling topic.unfenceTopicToResume after topic.close failed.

Documentation

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

Matching PR in forked repository

PR in forked repository: x

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

Jason918 · 2024-01-26T08:01:24Z

@poorbarcode Does this PR fix the issue mentioned in #21203 ?

poorbarcode · 2024-01-28T16:45:39Z

@Jason918

@poorbarcode Does this PR fix the issue mentioned in #21203 ?

Yes, the current PR also fixed the issue that #21203 tries to fix.

codelipenghui

Is it possible to add a test to cover this case?

And it looks like we can simplify the fix by adding a new method terminate() to the replicator so that we don't need to mix the closeProducer and closeReplicator logic.

poorbarcode · 2024-04-15T16:41:27Z

Rebase master

… an orphan replicator in the previous topic owner (apache#21946)

… an orphan replicator in the previous topic owner (#21946) (cherry picked from commit 4924052)

poorbarcode · 2024-05-07T12:18:54Z

Because there are too many conflicts and there are no new releases for 2.11, do not cherry-pick this PR into 2.11, note: the following PRs need to be cherry-picked together when performing cherry-picking.

[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946
[improve] [broker] Create partitioned topics automatically when enable topic level replication #22537
[fix] [broker] Part-2: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21948
[fix][broker] One topic can be closed multiple times concurrently #17524
[fix] [test] Fix flaky test ReplicatorTest #22594

… an orphan replicator in the previous topic owner (apache#21946) (cherry picked from commit 4924052) (cherry picked from commit 670aff0)

… an orphan replicator in the previous topic owner (apache#21946)

poorbarcode self-assigned this Jan 22, 2024

poorbarcode added this to the 3.3.0 milestone Jan 22, 2024

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 22, 2024

poorbarcode changed the title ~~[draft] [fix] [broker] Fix orphan replicator after unload topic~~ [draft] [fix] [broker] Replicator can not created successfully due to an orphan replicator in the previous topic owner Jan 22, 2024

poorbarcode changed the title ~~[draft] [fix] [broker] Replicator can not created successfully due to an orphan replicator in the previous topic owner~~ [draft] [fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner Jan 22, 2024

poorbarcode changed the title ~~[draft] [fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner~~ [draft] [fix] [broker] Part-3: Replicator can not created successfully due to an orphan replicator in the previous topic owner Jan 22, 2024

poorbarcode changed the title ~~[draft] [fix] [broker] Part-3: Replicator can not created successfully due to an orphan replicator in the previous topic owner~~ [draft] [fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner Jan 22, 2024

poorbarcode mentioned this pull request Jan 22, 2024

[fix] [broker] Part-2: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21948

Merged

4 tasks

poorbarcode changed the title ~~[draft] [fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner~~ [fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner Jan 22, 2024

poorbarcode requested review from Technoboy-, codelipenghui, lhotari and liangyepianzhou January 24, 2024 08:52

Technoboy- added the release/3.2.1 label Jan 25, 2024

Technoboy- reviewed Jan 26, 2024

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java Outdated Show resolved Hide resolved

Technoboy- reviewed Jan 26, 2024

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java Outdated Show resolved Hide resolved

poorbarcode mentioned this pull request Jan 28, 2024

[fix][broker] release orphan replicator after replicator is closed. #21203

Closed

4 tasks

poorbarcode force-pushed the fix/orphan_replicator_2 branch from 3eb5393 to 498ebec Compare January 28, 2024 16:53

poorbarcode requested a review from Technoboy- January 28, 2024 16:53

codelipenghui reviewed Jan 29, 2024

View reviewed changes

poorbarcode added 11 commits April 16, 2024 00:40

address comment

cdedb5b

add a method disconnect to calling close producer

febf6f0

address comments

5074e7a

remove unnecessary imports

6117922

Improve the lock and state changes

1ddafe4

fix bug

513b1f6

address comments

f908292

rename: stop -> disconnect

877452d

code formate

a474471

add a TODO comment

0675bda

fix test

0390ecf

poorbarcode force-pushed the fix/orphan_replicator_2 branch from 05de423 to 257f163 Compare April 15, 2024 16:41

poorbarcode force-pushed the fix/orphan_replicator_2 branch from 257f163 to 3bb81fa Compare April 19, 2024 12:17

fix tests

5793ca1

poorbarcode force-pushed the fix/orphan_replicator_2 branch from a42bd91 to 5793ca1 Compare April 22, 2024 16:50

poorbarcode merged commit 4924052 into apache:master Apr 23, 2024

Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request Apr 24, 2024

[fix] [broker] Part-1: Replicator can not created successfully due to…

041aa5f

… an orphan replicator in the previous topic owner (apache#21946)

lhotari mentioned this pull request Apr 25, 2024

[improve][ci] Disable test that causes OOME until the problem has been resolved #22586

Merged

4 tasks

poorbarcode added a commit that referenced this pull request May 7, 2024

[fix] [broker] Part-1: Replicator can not created successfully due to…

7a614c0

… an orphan replicator in the previous topic owner (#21946) (cherry picked from commit 4924052)

poorbarcode added the cherry-picked/branch-3.2 label May 7, 2024

poorbarcode added a commit that referenced this pull request May 7, 2024

[fix] [broker] Part-1: Replicator can not created successfully due to…

670aff0

… an orphan replicator in the previous topic owner (#21946) (cherry picked from commit 4924052)

poorbarcode added cherry-picked/branch-3.0 and removed release/2.10.6 release/3.1.3 labels May 7, 2024

poorbarcode mentioned this pull request Jun 28, 2024

[fix][broker] Replication stuck when partitions count between two clusters is not the same #22983

Merged

4 tasks

hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025

[fix] [broker] Part-1: Replicator can not created successfully due to…

8f93d6e

… an orphan replicator in the previous topic owner (apache#21946)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946

[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946

Uh oh!

poorbarcode commented Jan 22, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Jason918 commented Jan 26, 2024

Uh oh!

poorbarcode commented Jan 28, 2024

Uh oh!

codelipenghui left a comment

Uh oh!

poorbarcode commented Apr 15, 2024

Uh oh!

poorbarcode commented May 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946

[fix] [broker] Part-1: Replicator can not created successfully due to an orphan replicator in the previous topic owner #21946

Uh oh!

Conversation

poorbarcode commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

A case that hit this issue

Documentation

Matching PR in forked repository

Uh oh!

Uh oh!

Uh oh!

Jason918 commented Jan 26, 2024

Uh oh!

poorbarcode commented Jan 28, 2024

Uh oh!

codelipenghui left a comment

Choose a reason for hiding this comment

Uh oh!

poorbarcode commented Apr 15, 2024

Uh oh!

poorbarcode commented May 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

poorbarcode commented Jan 22, 2024 •

edited

Loading