[fix] [broker] Producer is blocked on creation because backlog exceeded on topic, when dedup is enabled and no producer is there #20951

heesung-sn · 2023-08-08T02:41:42Z

Motivation

As per backlog quota logic, Producer is blocked on creation when

backlog exceeded on topic
dedup is enabled
no producer is there.

Especially, producer creation is indefinitely blocked when there brokerDeduplicationEnabled=false(default=false), which disables periodic dedup map snapshots every 2mins.

When checking backlog size, the logic computes the backlog size against the slowest consumer on the topic. Unfortunately, the dedup system consumer(for the dedup map recovery) is usually behind the latest position -- the broker takes deup recovery map snapshot every 1000 entry(by default, brokerDeduplicationEntriesInterval) or every 2 mins(by default) when brokerDeduplicationEnabled is enabled(default is false), so before the next snapshot cycle, the dedup system consumer's MarkDeletePos is usually behind, and this can hold backlog more than the quota. As a result, the dedup-enabled topic could show more backlog size and block produce creations, especially when there is no producer and the time-based dedup-map snapshot is disabled, although all of the actual consumers' MarkDeletePos are up-to-date.

Modifications

Add advanceSlowestMessageDeduplicationCursor in the backlogQuotaCheck failure handler to advance the Slowest MessageDeduplication Cursor when the backlog quota exceeds the threshold.

Verifying this change

# local repro steps
bin/pulsar-admin tenants create my-tenant
bin/pulsar-admin namespaces create my-tenant/my-namespace-3 --bundles 5
bin/pulsar-admin namespaces set-backlog-quota my-tenant/my-namespace-3 -l 1K -p producer_request_hold
bin/pulsar-admin namespaces set-deduplication my-tenant/my-namespace-3 --enable

# on another console
bin/pulsar-client consume -s sub-read persistent://my-tenant/my-namespace-3/my-topic -p Earliest -n 0 --hide-content

# on another console, try multiple times until hit
bin/pulsar-perf produce persistent://my-tenant/my-namespace-3/my-topic -r 100 -bm 1 -mk random -n 1

Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:
Added the unit test to cover this logic change.

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

If the box was checked, please highlight the changes

Documentation

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

Matching PR in forked repository

PR in forked repository: heesung-sn#50

github-actions · 2023-08-08T02:42:06Z

@heesung-sn Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

poorbarcode · 2023-08-08T05:54:01Z

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

@@ -195,6 +195,10 @@ public class PersistentTopic extends AbstractTopic implements Topic, AddEntryCal
    private final TopicName shadowSourceTopic;

    static final String DEDUPLICATION_CURSOR_NAME = "pulsar.dedup";
+
+    public static boolean isDedupCursorName(String name) {
+        return name.equals(DEDUPLICATION_CURSOR_NAME);


Use DEDUPLICATION_CURSOR_NAME .equals(curosrName) is better, because the constant variable is never be null

poorbarcode

LGTM

poorbarcode · 2023-08-08T05:55:21Z

...r-broker/src/main/java/org/apache/pulsar/broker/service/persistent/MessageDeduplication.java

@@ -406,6 +410,11 @@ private void takeSnapshot(Position position) {
        if (log.isDebugEnabled()) {
            log.debug("[{}] Taking snapshot of sequence ids map", topic.getName());
        }
+
+        if (!snapshotTaking.compareAndSet(false, true)) {
+            return;


Since one updating cursor metadata task has been skipped(maybe the skipped one has a larger position), shall we print a log?

Seems like this snapshot can be called from other threads too. We better not overflow logs.

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

codelipenghui · 2023-08-14T02:02:52Z

@heesung-sn Please rebase to the master branch to resolve the CI issue.

heesung-sn · 2023-08-14T16:44:19Z

@heesung-sn Please rebase to the master branch to resolve the CI issue.

Done

congbobo184 · 2023-08-15T03:45:01Z

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java

        byte[] content = new byte[1024];
        Producer<byte[]> producer = createProducer(client, topic1);
+
+        admin.topicPolicies().setDeduplicationStatus(topic1, dedupTestSet);
+        Thread.sleep((TIME_TO_CHECK_BACKLOG_QUOTA + 1) * 1000);


better use admin to instead of sleep

congbobo184 · 2023-08-15T03:49:31Z

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java

        assertFalse(gotException, "unable to publish due to " + sendException);
+
+        gotException = false;


don't need this line

codelipenghui · 2023-08-16T13:18:45Z

@heesung-sn Is there anything blocking your committer process? I realized you still don't have permission for the pulsar repo.

…d on topic, when dedup is enabled and no producer is there

heesung-sn · 2023-08-24T01:04:04Z

@heesung-sn Is there anything blocking your committer process? I realized you still don't have permission for the pulsar repo.

I got the permission now. I merged this PR.

…ed on topic, when dedup is enabled and no producer is there (#20951) (cherry picked from commit 30073db)

…ed on topic, when dedup is enabled and no producer is there (apache#20951) (cherry picked from commit 30073db)

…ed on topic, when dedup is enabled and no producer is there (apache#20951) (cherry picked from commit 30073db) (cherry picked from commit f68589e)

github-actions bot added the doc-label-missing label Aug 8, 2023

heesung-sn force-pushed the fix-dedup-quota branch from e66a3a4 to d4ca29c Compare August 8, 2023 02:56

github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Aug 8, 2023

heesung-sn force-pushed the fix-dedup-quota branch 3 times, most recently from cacf511 to 0261b82 Compare August 8, 2023 05:44

poorbarcode reviewed Aug 8, 2023

View reviewed changes

poorbarcode approved these changes Aug 8, 2023

View reviewed changes

poorbarcode assigned heesung-sn Aug 8, 2023

poorbarcode added release/3.0.2 release/2.11.3 release/2.10.6 labels Aug 8, 2023

poorbarcode added this to the 3.2.0 milestone Aug 8, 2023

poorbarcode added the ready-to-test label Aug 8, 2023

poorbarcode added type/bug The PR fixed a bug or issue reported a bug and removed type/bug The PR fixed a bug or issue reported a bug labels Aug 8, 2023

codelipenghui reviewed Aug 8, 2023

View reviewed changes

heesung-sn force-pushed the fix-dedup-quota branch from 9180873 to f2c1805 Compare August 8, 2023 18:36

codelipenghui approved these changes Aug 9, 2023

View reviewed changes

codelipenghui added type/bug The PR fixed a bug or issue reported a bug area/broker labels Aug 9, 2023

lifepuzzlefun approved these changes Aug 12, 2023

View reviewed changes

heesung-sn force-pushed the fix-dedup-quota branch from f2c1805 to 1e76e6e Compare August 14, 2023 16:43

congbobo184 approved these changes Aug 15, 2023

View reviewed changes

codelipenghui added the category/functionality Some functions are not working such as get errors label Aug 21, 2023

heesung-sn added 2 commits August 23, 2023 09:40

[broker][fix] Producer is blocked on creation because backlog exceede…

05f8da1

…d on topic, when dedup is enabled and no producer is there

resolved comments

f87b86e

heesung-sn force-pushed the fix-dedup-quota branch from 1e76e6e to f87b86e Compare August 23, 2023 16:41

heesung-sn merged commit 30073db into apache:master Aug 24, 2023
44 of 45 checks passed

poorbarcode pushed a commit that referenced this pull request Aug 29, 2023

[fix] [broker] Producer is blocked on creation because backlog exceed…

f68589e

…ed on topic, when dedup is enabled and no producer is there (#20951) (cherry picked from commit 30073db)

poorbarcode added the cherry-picked/branch-3.0 label Aug 29, 2023

shibd added the cherry-picked/branch-2.11 label Oct 22, 2023

shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 22, 2023

[fix] [broker] Producer is blocked on creation because backlog exceed…

17c3890

…ed on topic, when dedup is enabled and no producer is there (apache#20951) (cherry picked from commit 30073db)

shibd pushed a commit to shibd/pulsar that referenced this pull request Oct 24, 2023

[fix] [broker] Producer is blocked on creation because backlog exceed…

ebdf7dc

…ed on topic, when dedup is enabled and no producer is there (apache#20951) (cherry picked from commit 30073db)

shibd mentioned this pull request Oct 24, 2023

[Don't merge][cherry-pick][branch-2.11] Cherry-pick some PR to branch-2.11 #21427

Closed

14 tasks

lhotari added release/2.10.7 and removed release/2.10.6 labels Mar 5, 2024

heesung-sn deleted the fix-dedup-quota branch April 2, 2024 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] [broker] Producer is blocked on creation because backlog exceeded on topic, when dedup is enabled and no producer is there #20951

[fix] [broker] Producer is blocked on creation because backlog exceeded on topic, when dedup is enabled and no producer is there #20951

heesung-sn commented Aug 8, 2023 •

edited

github-actions bot commented Aug 8, 2023

poorbarcode Aug 8, 2023

heesung-sn Aug 8, 2023

poorbarcode left a comment

poorbarcode Aug 8, 2023

heesung-sn Aug 8, 2023

codelipenghui commented Aug 14, 2023

heesung-sn commented Aug 14, 2023

congbobo184 Aug 15, 2023

congbobo184 Aug 15, 2023

codelipenghui commented Aug 16, 2023

heesung-sn commented Aug 24, 2023

		assertFalse(gotException, "unable to publish due to " + sendException);

		gotException = false;

[fix] [broker] Producer is blocked on creation because backlog exceeded on topic, when dedup is enabled and no producer is there #20951

[fix] [broker] Producer is blocked on creation because backlog exceeded on topic, when dedup is enabled and no producer is there #20951

Conversation

heesung-sn commented Aug 8, 2023 • edited

Motivation

Modifications

Verifying this change

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

Documentation

Matching PR in forked repository

github-actions bot commented Aug 8, 2023

poorbarcode Aug 8, 2023

Choose a reason for hiding this comment

heesung-sn Aug 8, 2023

Choose a reason for hiding this comment

poorbarcode left a comment

Choose a reason for hiding this comment

poorbarcode Aug 8, 2023

Choose a reason for hiding this comment

heesung-sn Aug 8, 2023

Choose a reason for hiding this comment

codelipenghui commented Aug 14, 2023

heesung-sn commented Aug 14, 2023

congbobo184 Aug 15, 2023

Choose a reason for hiding this comment

congbobo184 Aug 15, 2023

Choose a reason for hiding this comment

codelipenghui commented Aug 16, 2023

heesung-sn commented Aug 24, 2023

heesung-sn commented Aug 8, 2023 •

edited