Skip to content

[fix][broker] In the trimDeletedEntries method, release the removed entry - #18305

Merged
codelipenghui merged 2 commits into
apache:masterfrom
lordcheng10:fix_buffer_release
Nov 3, 2022
Merged

[fix][broker] In the trimDeletedEntries method, release the removed entry#18305
codelipenghui merged 2 commits into
apache:masterfrom
lordcheng10:fix_buffer_release

Conversation

@lordcheng10

@lordcheng10 lordcheng10 commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Motivation

In the trimDeletedEntries method, the removed entry is not released:

public void trimDeletedEntries(List<Entry> entries) {
entries.removeIf(entry -> ((PositionImpl) entry.getPosition()).compareTo(markDeletePosition) <= 0
|| individualDeletedMessages.contains(entry.getLedgerId(), entry.getEntryId()));
}

protected synchronized boolean trySendMessagesToConsumers(ReadType readType, List<Entry> entries) {
if (needTrimAckedMessages()) {
cursor.trimDeletedEntries(entries);
}

Documentation

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

Matching PR in forked repository

PR in forked repository: lordcheng10#38

@lordcheng10

Copy link
Copy Markdown
Contributor Author

@codelipenghui @Jason918 PTAL,thanks!

@codelipenghui codelipenghui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch!

@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codecov-commenter

codecov-commenter commented Nov 3, 2022

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.54%. Comparing base (0866c3a) to head (56d0e6e).
⚠️ Report is 3404 commits behind head on master.

Files with missing lines Patch % Lines
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18305      +/-   ##
============================================
- Coverage     38.97%   38.54%   -0.44%     
- Complexity     8311     8868     +557     
============================================
  Files           683      747      +64     
  Lines         67325    72197    +4872     
  Branches       7217     7754     +537     
============================================
+ Hits          26239    27825    +1586     
- Misses        38079    41261    +3182     
- Partials       3007     3111     +104     
Flag Coverage Δ
unittests 38.54% <0.00%> (-0.44%) ⬇️

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

Files with missing lines Coverage Δ
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 36.87% <0.00%> (-0.03%) ⬇️

... and 127 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@lordcheng10

Copy link
Copy Markdown
Contributor Author

@AnonHxy @Jason918 @HQebupt PTAL,thanks!

@lordcheng10

Copy link
Copy Markdown
Contributor Author

@Technoboy- PTAL,thanks!

@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

2 similar comments
@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@lordcheng10

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@coderzc

coderzc commented Nov 3, 2022

Copy link
Copy Markdown
Member

/pulsarbot run-failure-checks

Pulsar CI Flaky is not necessary.

@codelipenghui
codelipenghui merged commit 79a3f85 into apache:master Nov 3, 2022
congbobo184 pushed a commit that referenced this pull request Nov 8, 2022
@congbobo184 congbobo184 added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Nov 8, 2022
congbobo184 pushed a commit that referenced this pull request Nov 26, 2022
aloyszhang pushed a commit to aloyszhang/pulsar that referenced this pull request Nov 30, 2022
…ntry (apache#18305) (merge request !161)

Squash merge branch 'release-2.8.1.4-fix_mem_leak' into 'release-2.8.1.4'
Fixes #<xyz>

### Motivation
[fix][broker] In the trimDeletedEntries method, release the removed entry (apache#18305)
liangyepianzhou pushed a commit that referenced this pull request Dec 14, 2022
aloyszhang pushed a commit to aloyszhang/pulsar that referenced this pull request Jan 9, 2023
…ntry (apache#18305) (merge request !161)

Squash merge branch 'release-2.8.1.4-fix_mem_leak' into 'release-2.8.1.4'
Fixes #<xyz>

### Motivation
[fix][broker] In the trimDeletedEntries method, release the removed entry (apache#18305)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 10, 2023
…ntry (apache#18305)

(cherry picked from commit 79a3f85)
(cherry picked from commit 4eb93ec)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 11, 2023
…ntry (apache#18305)

(cherry picked from commit 79a3f85)
(cherry picked from commit 4eb93ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants