[fix][broker] fix replicated subscriptions for transactional messages by thetumbled · Pull Request #22452 · apache/pulsar

thetumbled · 2024-04-07T06:31:04Z

Motivation

In non-transactional production, we update the LastDataMessagePublishedTimestamp when the message is persisted successfully. But in transactional production, we do not update LastDataMessagePublishedTimestamp, which will impact the feature ReplicatedSubscription.

Modifications

Update the LastDataMessagePublishedTimestamp when the max read position move forward.

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

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: thetumbled#45

thetumbled · 2024-04-07T06:33:24Z

PTAL, thanks. @BewareMyPower @poorbarcode @codelipenghui @liangyepianzhou

BewareMyPower

Could you add a test for it?

thetumbled · 2024-04-08T07:36:43Z

Could you add a test for it?

Added, PTAL, thanks.

liangyepianzhou · 2024-04-08T07:39:13Z

@thetumbled You need to add a test related to transaction, right?

thetumbled · 2024-04-08T07:41:21Z

@thetumbled You need to add a test related to transaction, right?

testUpdateLastDataMessagePublishedTimestampForTransactionalPublish()

thetumbled · 2024-05-09T03:44:14Z

/pulsarbot rerun-failure-checks

thetumbled · 2024-05-09T06:08:46Z

/pulsarbot rerun-failure-checks

congbobo184 · 2024-05-09T11:37:57Z

Error: org.apache.pulsar.broker.service.ReplicatorSubscriptionWithTransactionTest.testReplicatedSubscriptionWhenReplicatorProducerIsClosed Time elapsed: 12.567 s <<< FAILURE!
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.apache.pulsar.broker.service.ReplicatorSubscriptionTest expected object to not be null within 10 seconds.

Pulsar CI / CI - Unit - Brokers - Broker Group 1 (pull_request) test is not stable, please check the reason

thetumbled · 2024-05-09T12:55:42Z

Error: org.apache.pulsar.broker.service.ReplicatorSubscriptionWithTransactionTest.testReplicatedSubscriptionWhenReplicatorProducerIsClosed Time elapsed: 12.567 s <<< FAILURE!
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.apache.pulsar.broker.service.ReplicatorSubscriptionTest expected object to not be null within 10 seconds.

Pulsar CI / CI - Unit - Brokers - Broker Group 1 (pull_request) test is not stable, please check the reason

I have fixed the test code, please help to trigger the CI again, thanks.

thetumbled · 2024-05-10T01:37:13Z

/pulsarbot rerun-failure-checks

thetumbled · 2024-05-10T02:15:36Z

/pulsarbot rerun-failure-checks

thetumbled · 2024-05-10T02:52:37Z

/pulsarbot rerun-failure-checks

dao-jun · 2024-05-13T03:32:46Z

@poorbarcode any more change requests?

thetumbled · 2024-05-13T06:36:30Z

PTAL, thanks. @poorbarcode

lhotari · 2024-05-14T03:42:21Z

cherry-picking this to branch-3.0 . it looks like #21816 and #22656 need to be cherry-picked before this one to reduce merge conflicts.

thetumbled · 2024-05-14T03:53:46Z

cherry-picking this to branch-3.0 . it looks like #21816 and #22656 need to be cherry-picked before this one to reduce merge conflicts.

Hi, Lari. You can cherry pick #22656 into branch-3.0 without conflict, and i can help to cherry pick this pr if there are any conflict.

lhotari · 2024-05-14T03:55:10Z

I'm giving up on cherry-picking #21816 since that's too large change. I'll check what the conflicts are when applying only #22656 and #22452.

…#22452) (cherry picked from commit 9fd1b61)

…#22452) (cherry picked from commit 9fd1b61) # Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java

…apache#22452) (cherry picked from commit 9fd1b61) (cherry picked from commit e300fbd)

…#22452)

…apache#22452)

fix not update LastDataMessagePublishedTimestamp.

5d557e8

github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Apr 7, 2024

thetumbled mentioned this pull request Apr 7, 2024

[fix][broker] fix replicated subscriptions for transactional messages thetumbled/pulsar#45

Closed

dao-jun reviewed Apr 7, 2024

View reviewed changes

Comment thread ...r/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java Outdated

thetumbled added 3 commits April 7, 2024 16:06

fix checkstyle.

1f92882

change.

2524031

fix.

566ddc0

dao-jun reviewed Apr 7, 2024

View reviewed changes

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

BewareMyPower reviewed Apr 8, 2024

View reviewed changes

thetumbled added 2 commits April 8, 2024 15:32

add test code.

8c913ff

fix checkstyle.

51597c2

thetumbled added 2 commits April 8, 2024 15:47

change test code.

cc6d062

nit.

6fae3a3

codelipenghui reviewed Apr 8, 2024

View reviewed changes

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

thetumbled mentioned this pull request Apr 8, 2024

[fix] [broker] fix startNewSnapshot when lastDataMessagePublishedTimestamp not change. #22461

Closed

15 tasks

lhotari reviewed Apr 8, 2024

View reviewed changes

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

thetumbled changed the title ~~[fix] [broker] fix not updating LastDataMessagePublishedTimestamp in transactional production.~~ [fix][broker] fix replicated subscriptions for transactional messages Apr 9, 2024

thetumbled force-pushed the Fix_lastDataMessagePublishedTimestamp branch from e27fd14 to 6fae3a3 Compare April 9, 2024 08:00

add test.

6747e45

lhotari reviewed Apr 9, 2024

View reviewed changes

Comment thread ...r/src/test/java/org/apache/pulsar/broker/service/TransactionalReplicateSubscriptionTest.java

thetumbled added 6 commits April 9, 2024 19:51

add license header.

d51980e

add test code.

ccc39dc

refactor end-to-end test code.

b8fd85d

clean up max read position.

59c23ab

support max read position callback.

2c152c9

fix.

c965955

fix conflict.

5d1c2ba

congbobo184 reviewed May 9, 2024

View reviewed changes

Comment thread ...r/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java

thetumbled added 2 commits May 9, 2024 20:50

fix test code.

2d24e34

add checkIfNoSnapshot check back.

2f416f0

congbobo184 approved these changes May 10, 2024

View reviewed changes

poorbarcode approved these changes May 13, 2024

View reviewed changes

poorbarcode merged commit 9fd1b61 into apache:master May 13, 2024

lhotari pushed a commit that referenced this pull request May 14, 2024

[fix][broker] fix replicated subscriptions for transactional messages (…

e300fbd

…#22452) (cherry picked from commit 9fd1b61)

lhotari added the cherry-picked/branch-3.0 label May 14, 2024

lhotari added the cherry-picked/branch-3.2 label May 14, 2024

nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 15, 2024

[fix][broker] fix replicated subscriptions for transactional messages (…

d7b24e9

…apache#22452) (cherry picked from commit 9fd1b61) (cherry picked from commit e300fbd)

srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024

[fix][broker] fix replicated subscriptions for transactional messages (…

99c2a9d

…apache#22452) (cherry picked from commit 9fd1b61) (cherry picked from commit e300fbd)

coderzc added the release/3.3.1 label Jun 3, 2024

Technoboy- added the cherry-picked/branch-3.3 label Jun 6, 2024

Technoboy- pushed a commit that referenced this pull request Jun 6, 2024

[fix][broker] fix replicated subscriptions for transactional messages (…

dd408d4

…#22452)

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

[fix][broker] fix replicated subscriptions for transactional messages (…

328d562

…apache#22452)

lhotari mentioned this pull request Jan 14, 2026

msgBacklog, storageSize, backlogSize metrics keep increasing after stopping all consumers #16606

Open

Conversation

thetumbled commented Apr 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

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

Documentation

Matching PR in forked repository

Uh oh!

thetumbled commented Apr 7, 2024

Uh oh!

Uh oh!

Uh oh!

BewareMyPower left a comment

Choose a reason for hiding this comment

Uh oh!

thetumbled commented Apr 8, 2024

Uh oh!

liangyepianzhou commented Apr 8, 2024

Uh oh!

thetumbled commented Apr 8, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thetumbled commented May 9, 2024

Uh oh!

thetumbled commented May 9, 2024

Uh oh!

Uh oh!

congbobo184 commented May 9, 2024

Uh oh!

thetumbled commented May 9, 2024

Uh oh!

thetumbled commented May 10, 2024

Uh oh!

thetumbled commented May 10, 2024

Uh oh!

thetumbled commented May 10, 2024

Uh oh!

dao-jun commented May 13, 2024

Uh oh!

thetumbled commented May 13, 2024

Uh oh!

lhotari commented May 14, 2024

Uh oh!

thetumbled commented May 14, 2024

Uh oh!

lhotari commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

thetumbled commented Apr 7, 2024 •

edited

Loading