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.

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

[fix][broker] Fix incorrect number of read compacted entries #20978

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented Aug 11, 2023

Motivation

Currently, the number of read compacted entries is wrong, which is one more than the specified number.

Modifications

Fix incorrect number of read compacted entries.

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.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 11, 2023
@coderzc coderzc changed the title [fix][bug] Fix incorrect number of read compacted entries [fix][broker] Fix incorrect number of read compacted entries Aug 11, 2023
@coderzc coderzc self-assigned this Aug 11, 2023
@coderzc coderzc added type/bug The PR fixed a bug or issue reported a bug area/broker labels Aug 11, 2023
@coderzc coderzc added this to the 3.2.0 milestone Aug 11, 2023
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codecov-commenter
Copy link

Codecov Report

Merging #20978 (2748b99) into master (9862884) will increase coverage by 39.60%.
Report is 7 commits behind head on master.
The diff coverage is 50.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20978       +/-   ##
=============================================
+ Coverage     33.53%   73.13%   +39.60%     
- Complexity    12174    32253    +20079     
=============================================
  Files          1621     1875      +254     
  Lines        126919   139407    +12488     
  Branches      13851    15329     +1478     
=============================================
+ Hits          42561   101955    +59394     
+ Misses        78745    29400    -49345     
- Partials       5613     8052     +2439     
Flag Coverage Δ
inttests 24.09% <50.00%> (-0.19%) ⬇️
systests 25.10% <0.00%> (?)
unittests 72.42% <50.00%> (+40.37%) ⬆️

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

Files Changed Coverage Δ
...g/apache/pulsar/compaction/CompactedTopicImpl.java 63.90% <0.00%> (+4.51%) ⬆️
...ulsar/compaction/PulsarTopicCompactionService.java 76.47% <100.00%> (ø)

... and 1520 files with indirect coverage changes

@coderzc coderzc merged commit 63d9eaf into apache:master Aug 14, 2023
50 checks passed
@coderzc coderzc deleted the fix_compacted_num branch August 14, 2023 10:38
coderzc added a commit that referenced this pull request Aug 14, 2023
coderzc added a commit that referenced this pull request Aug 14, 2023
coderzc added a commit that referenced this pull request Aug 14, 2023
coderzc added a commit that referenced this pull request Aug 14, 2023
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.

5 participants