[fix] [client] Messages lost due to TopicListWatcher reconnect #21853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

Technoboy- merged 12 commits into apache:master from poorbarcode:fix/topic_watcher_2

Jan 8, 2024

Contributor

poorbarcode commented Jan 4, 2024

Motivation

Issue

Start a consumer subscribe with regexp public/default/*
Create a new topic public/default/tp-1
Send messages to the new topic
Consumers will not receive any messages, and the messages will be deleted due to there are no subscription.

Root cause

Case-1:

Time	Thread `start consumer`	Thread `create topic`
1	Start subscribe
2	Start to register `TopicListWatcher`	Create new topic
3		Topic created
4		Since there are no listeners registered, there is no need to notice clients
5	`TopicListWatcher` registered, the consumer will not receive any messages from the new topic

Case-2:

Time	Thread `TopicListWatcher reconnect`	Thread `create topic`
1	Start reconnect
2		Create new topic
3		Topic created
4		Since there are no listeners registered, there is no need to notice clients
5	`TopicListWatcher` reconnected
6	The consumer will not receive any messages from the new topic

This issue only affects the releases >=3.1, because before 3.1 there is a supplementary mechanism scheduled recheckPatternTimeout, it makes the issues will not occur. This supplementary mechanism was removed at 3.1, see #20779

Modifications

After TopicListWatcher reconnected, re-check the topics list.

Documentation

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

Matching PR in forked repository

PR in forked repository: x


          [fix] [client] Messages lost due to TopicListWatcher reconnect

4527adc

(cherry picked from commit f455b8d)

poorbarcode requested review from Technoboy-, codelipenghui, liangyepianzhou and shibd

January 4, 2024 12:07

poorbarcode self-assigned this

github-actions bot added the doc-not-needed label

poorbarcode added type/bug category/reliability release/3.1.3 and removed doc-not-needed labels

poorbarcode added this to the 3.3.0 milestone

github-actions bot added the doc-not-needed label

RobertIndie requested changes

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated Show resolved Hide resolved

poorbarcode added 2 commits

January 5, 2024 18:15

ed1d224


          address comments

495fb38

poorbarcode requested a review from RobertIndie

January 5, 2024 10:21

RobertIndie approved these changes

View reviewed changes

Technoboy- reviewed

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated Show resolved Hide resolved

Technoboy- reviewed

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated

+                  private final CompletableFuture<TopicListWatcher> watcherFuture = new CompletableFuture<>();
                   protected NamespaceName namespaceName;
                   private volatile Timeout recheckPatternTimeout = null;
+                  private volatile AtomicReference<Timeout> retryRecheckPatternTask = new AtomicReference<>();

Contributor

Technoboy- Jan 5, 2024

recheckPatternTask is better

Technoboy- reviewed

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated Show resolved Hide resolved

Technoboy- reviewed

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated Show resolved Hide resolved


          simplify logic

ed1eaec

lhotari reviewed

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java Outdated Show resolved Hide resolved

lhotari reviewed

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/client/impl/PatternTopicsConsumerImplTest.java Show resolved Hide resolved

poorbarcode added 3 commits

January 8, 2024 16:51


          address comments

34eeff2


          address comments

78971f2


          address comments

42cda95

Technoboy- reviewed

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Outdated Show resolved Hide resolved


          address comments

7616f7c

lhotari approved these changes

View reviewed changes

Member

lhotari left a comment

LGTM

Technoboy- approved these changes

View reviewed changes

Technoboy- requested review from BewareMyPower, Demogorgon314, coderzc and heesung-sohn

January 8, 2024 09:00

Demogorgon314 reviewed

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java Outdated Show resolved Hide resolved

pulsar-client/src/main/java/org/apache/pulsar/client/impl/PatternMultiTopicsConsumerImpl.java Show resolved Hide resolved


          address comments

fb40dd7

Demogorgon314 approved these changes

View reviewed changes

coderzc approved these changes

View reviewed changes

poorbarcode added 2 commits

January 8, 2024 17:41


          fix the issue of thread-safety

59e7ef9


          seperate the lockj

aec1eba

Demogorgon314 approved these changes

View reviewed changes


          seperate the lock

Technoboy- merged commit 042e769 into apache:master

Technoboy- pushed a commit that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (#21853)

fd62df8

Technoboy- modified the milestones: 3.3.0, 3.2.0

poorbarcode added a commit that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (#21853)

3fc7cb4

(cherry picked from commit 042e769)

poorbarcode added the cherry-picked/branch-3.1 label

poorbarcode added a commit that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (#21853)

4ef5132

(cherry picked from commit 042e769)

poorbarcode added cherry-picked/branch-3.0 release/3.0.3 labels

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (apach…

de51eba

…e#21853)

(cherry picked from commit 042e769)
(cherry picked from commit 3fc7cb4)

mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (apach…

202dc05

…e#21853)

(cherry picked from commit 042e769)
(cherry picked from commit 3fc7cb4)

poorbarcode added a commit that referenced this pull request


          [fix] [client] Messages lost due to TopicListWatcher reconnect (#21853)

(cherry picked from commit 042e769)

poorbarcode added cherry-picked/branch-2.11 release/2.11.5 labels

shibd mentioned this pull request

[fix][client] Pattern subscription regression when broker-side evaluation is disabled #24104

Merged

4 tasks

lhotari mentioned this pull request

[feat] PIP-442: Add memory limits for CommandGetTopicsOfNamespace #24833

Merged

5 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Technoboy- Technoboy- approved these changes

Demogorgon314 Demogorgon314 approved these changes

lhotari lhotari approved these changes

RobertIndie RobertIndie approved these changes

coderzc coderzc approved these changes

codelipenghui Awaiting requested review from codelipenghui

shibd Awaiting requested review from shibd

liangyepianzhou Awaiting requested review from liangyepianzhou

BewareMyPower Awaiting requested review from BewareMyPower

heesung-sohn Awaiting requested review from heesung-sohn

Labels

category/reliability cherry-picked/branch-2.11 cherry-picked/branch-3.0 cherry-picked/branch-3.1 doc-not-needed release/2.11.5 release/3.0.3 release/3.1.3 type/bug