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

[improve] [broker] PIP-299-part-1: Stop dispatch messages if the individual acks will be lost in the persistent storage #21423

Merged
merged 22 commits into from
Dec 27, 2023
Merged

[improve] [broker] PIP-299-part-1: Stop dispatch messages if the individual acks will be lost in the persistent storage #21423

merged 22 commits into from
Dec 27, 2023

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Oct 23, 2023 •
edited
Loading

Motivation & Modifications

The part 1 of PIP-299: the implementation of "Stop dispatch messages if the individual acks will be lost in the persistent storage"

Subsequent PRs:

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode changed the title [improve broker] Stop dispatch messages if the individual acks will b… [improve broker] Stop dispatch messages if the individual acks will be lost in the persistent storage Oct 23, 2023
@poorbarcode poorbarcode mentioned this pull request Oct 23, 2023
Merged
4 tasks
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 23, 2023
@poorbarcode poorbarcode self-assigned this Oct 23, 2023
@poorbarcode poorbarcode added the category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost label Oct 23, 2023
@poorbarcode poorbarcode added this to the 3.2.0 milestone Oct 23, 2023
@poorbarcode poorbarcode reopened this Oct 23, 2023
@poorbarcode poorbarcode changed the title [improve broker] Stop dispatch messages if the individual acks will be lost in the persistent storage [improve] [broker] [part-1] Stop dispatch messages if the individual acks will be lost in the persistent storage Oct 23, 2023
@poorbarcode poorbarcode changed the title [improve] [broker] [part-1] Stop dispatch messages if the individual acks will be lost in the persistent storage [improve] [broker] PIP-299-part-2: Stop dispatch messages if the individual acks will be lost in the persistent storage Oct 23, 2023
@poorbarcode poorbarcode changed the title [improve] [broker] PIP-299-part-2: Stop dispatch messages if the individual acks will be lost in the persistent storage [improve] [broker] PIP-299-part-1: Stop dispatch messages if the individual acks will be lost in the persistent storage Oct 23, 2023
Technoboy-
Technoboy- reviewed Dec 21, 2023
View reviewed changes
...pulsar/broker/service/persistent/BlockDispatcherSignatureOnCursorDataCanNotFullyPersist.java Outdated
* We just call {@link PersistentDispatcherMultipleConsumers#readMoreEntries()} after the dispatching has been
* paused at least once earlier.
*/
private volatile boolean markerAtLeastPausedOnce;
Copy link
Contributor

Choose a reason for hiding this comment

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

markerPausedAtLeastOnce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed

@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 25, 2023 •
edited
Loading

Codecov Report

Attention: 28 lines in your changes are missing coverage. Please review.

Comparison is base (181b20b) 73.42% compared to head (0cd3718) 73.57%.
Report is 16 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21423      +/-   ##
============================================
+ Coverage     73.42%   73.57%   +0.15%     
+ Complexity    32791    32280     -511     
============================================
  Files          1897     1858      -39     
  Lines        140633   138098    -2535     
  Branches      15491    15135     -356     
============================================
- Hits         103254   101601    -1653     
+ Misses        29306    28636     -670     
+ Partials       8073     7861     -212
Flag Coverage Δ
inttests 24.18% <25.80%> (+<0.01%) ⬆️
systests 23.67% <20.96%> (-0.96%) ⬇️
unittests 72.87% <54.83%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 79.31% <100.00%> (+0.12%) ⬆️
...rg/apache/pulsar/broker/service/AbstractTopic.java 87.92% <100.00%> (+0.18%) ⬆️
...a/org/apache/pulsar/broker/service/Dispatcher.java 57.89% <100.00%> (+2.33%) ⬆️
...sar/broker/service/persistent/PersistentTopic.java 78.84% <100.00%> (+0.16%) ⬆️
...r/common/policies/data/HierarchyTopicPolicies.java 100.00% <100.00%> (ø)
...che/pulsar/common/policies/data/TopicPolicies.java 58.73% <100.00%> (+0.66%) ⬆️
...a/org/apache/bookkeeper/mledger/ManagedCursor.java 40.00% <0.00%> (-2.86%) ⬇️
...tent/PersistentDispatcherSingleActiveConsumer.java 70.49% <0.00%> (-1.38%) ⬇️
...ker/service/persistent/PersistentSubscription.java 77.05% <61.11%> (-0.21%) ⬇️
...sistent/PersistentDispatcherMultipleConsumers.java 74.15% <46.66%> (-1.40%) ⬇️

... and 135 files with indirect coverage changes

liangyepianzhou
liangyepianzhou approved these changes Dec 26, 2023
View reviewed changes
Copy link
Contributor

@liangyepianzhou liangyepianzhou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good work!

@poorbarcode poorbarcode merged commit 8ff32db into apache:master Dec 27, 2023
47 checks passed
@poorbarcode poorbarcode mentioned this pull request Jan 2, 2024
Merged
7 tasks
@poorbarcode poorbarcode mentioned this pull request Jan 10, 2024
Merged
7 tasks
@Technoboy- Technoboy- mentioned this pull request Jan 18, 2024
Merged
8 tasks
@poorbarcode poorbarcode mentioned this pull request May 17, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@liangyepianzhou liangyepianzhou liangyepianzhou approved these changes

@gaoran10 gaoran10 Awaiting requested review from gaoran10

@coderzc coderzc Awaiting requested review from coderzc

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

Assignees

@poorbarcode poorbarcode

Labels
category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
6 participants
@poorbarcode @codecov-commenter @Technoboy- @codelipenghui @coderzc @liangyepianzhou