Skip to content
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

Trim messages which less than mark delete position for message redelivery #5378

Merged
merged 3 commits into from
Oct 30, 2019
Merged

Trim messages which less than mark delete position for message redelivery #5378

merged 3 commits into from
Oct 30, 2019

Conversation

codelipenghui
Copy link
Contributor

Master Issue: #5377

Motivation

Fix #5377

Modifications

Trim messages which less than mark delete position for message redelivery

Verifying this change

Add new unit test for this change

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@codelipenghui codelipenghui self-assigned this Oct 12, 2019
@codelipenghui codelipenghui added this to the 2.4.2 milestone Oct 12, 2019
@codelipenghui
Copy link
Contributor Author

run java8 tests
run integration tests

@codelipenghui
Copy link
Contributor Author

run integration tests

merlimat
merlimat reviewed Oct 14, 2019
View reviewed changes
...broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java Outdated
}
}

private List<PositionImpl> trimByMarkDeletePosition(List<PositionImpl> positions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is applying this filtering which generates a lot of temp objects each time these redeliveries are done. Why not clear the map when the clear-backlog or seek commands are issued?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with removeIf(), please take a look.

@codelipenghui
Copy link
Contributor Author

retest this please

@codelipenghui
Copy link
Contributor Author

run java8 tests

@codelipenghui
Copy link
Contributor Author

run cpp tests

@jiazhai
Copy link
Member

jiazhai commented Oct 14, 2019

run cpp tests

@sijie
Copy link
Member

sijie commented Oct 14, 2019

run cpp tests
run java8 tests

@codelipenghui
Copy link
Contributor Author

retest this please

1 similar comment
@codelipenghui
Copy link
Contributor Author

retest this please

@codelipenghui
Copy link
Contributor Author

run cpp tests
run integration tests

@codelipenghui
Copy link
Contributor Author

run java8 tests
run integration tests

@codelipenghui
Copy link
Contributor Author

run integration tests

2 similar comments
@codelipenghui
Copy link
Contributor Author

run integration tests

@codelipenghui
Copy link
Contributor Author

run integration tests

@codelipenghui
Copy link
Contributor Author

run java8 tests

@codelipenghui
Copy link
Contributor Author

run integration Tests

@wolfstudy
Copy link
Member

run java8 tests

1 similar comment
@wolfstudy
Copy link
Member

run java8 tests

@wolfstudy
Copy link
Member

org.apache.pulsar.broker.service.PeerReplicatorTest.testPeerClusterInReplicationClusterListChange

run java8 tests

@wolfstudy
Copy link
Member

org.apache.pulsar.client.api.BrokerServiceLookupTest.setup
org.apache.pulsar.client.api.SimpleSchemaTest.setup
org.apache.pulsar.client.api.SubscriptionMessageDispatchThrottlingTest.setup
org.apache.pulsar.client.api.v1.V1_ProducerConsumerTest.setup
org.apache.pulsar.functions.worker.PulsarFunctionE2ESecurityTest.testAuthorizationWithAnonymousUser

run java8 tests

@wolfstudy
Copy link
Member

run java8 tests

@wolfstudy
Copy link
Member

org.apache.pulsar.broker.admin.AdminApiOffloadTest.testOffloadV1
org.apache.pulsar.functions.worker.PulsarFunctionE2ESecurityTest.testAuthorizationWithAnonymousUser

run java8 tests

@wolfstudy wolfstudy merged commit b11211f into apache:master Oct 30, 2019
wolfstudy pushed a commit that referenced this pull request Nov 20, 2019
@codelipenghui @wolfstudy
Trim messages which less than mark delete position for message redeli…
ae864d5 
…very (#5378)

* Trim messages which less than mark delete position for message redelivery.


(cherry picked from commit b11211f)
@codelipenghui codelipenghui deleted the redelivery_trim_mark_deleted_position branch May 19, 2021 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merlimat merlimat merlimat left review comments

@sijie sijie sijie approved these changes

@jiazhai jiazhai jiazhai approved these changes

@aahmed-se aahmed-se aahmed-se approved these changes

@rdhabalia rdhabalia Awaiting requested review from rdhabalia

@massakam massakam Awaiting requested review from massakam

Assignees

@codelipenghui codelipenghui

Labels
area/broker
Projects
None yet
Milestone
2.4.2
Development

Successfully merging this pull request may close these issues.

Old message continue redeliver after clear backlog.
6 participants
@codelipenghui @jiazhai @sijie @wolfstudy @merlimat @aahmed-se