Skip to content

[improve][broker] GetMessageById add RedeliveryCount for output#15095

Merged
tisonkun merged 2 commits intoapache:masterfrom
leizhiyuan:issue/15086
Dec 10, 2022
Merged

[improve][broker] GetMessageById add RedeliveryCount for output#15095
tisonkun merged 2 commits intoapache:masterfrom
leizhiyuan:issue/15086

Conversation

@leizhiyuan
Copy link
Contributor

@leizhiyuan leizhiyuan commented Apr 9, 2022

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #15086

(or if this PR is one task of a github issue, please add Master Issue: #<xyz> to link to the master issue.)

Master Issue: #15086

Motivation

GetMessageById add RedeliveryCount for output
fix typo

Explain here the context, and why you're making that change. What is the problem you're trying to solve.

Modifications

Describe the modifications you've done.

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

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): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-added
    (Docs have been already added)

@github-actions
Copy link

github-actions bot commented Apr 9, 2022

@leizhiyuan:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions
Copy link

github-actions bot commented Apr 9, 2022

@leizhiyuan:Thanks for providing doc info!

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 9, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone Apr 10, 2022
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Jun 17, 2022
@Jason918
Copy link
Contributor

/pulsarbot run-failure-checks

@github-actions github-actions bot removed the Stale label Jun 19, 2022
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Jul 20, 2022
@Jason918
Copy link
Contributor

/pulsarbot run-failure-checks

@github-actions github-actions bot removed the Stale label Jul 25, 2022
@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.12.0 Jul 26, 2022
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Aug 25, 2022
@tisonkun tisonkun changed the title feat: GetMessageById add RedeliveryCount for output, fix typo [improve][broker] GetMessageById add RedeliveryCount for output Dec 10, 2022
@github-actions github-actions bot removed the doc-not-needed Your PR changes do not impact docs label Dec 10, 2022
@github-actions
Copy link

@leizhiyuan Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2022

Codecov Report

Merging #15095 (f46dbad) into master (d30e86c) will decrease coverage by 3.24%.
The diff coverage is 12.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #15095      +/-   ##
============================================
- Coverage     47.92%   44.68%   -3.25%     
- Complexity    10679    10694      +15     
============================================
  Files           703      763      +60     
  Lines         68831    73596    +4765     
  Branches       7378     7912     +534     
============================================
- Hits          32988    32884     -104     
- Misses        32119    37013    +4894     
+ Partials       3724     3699      -25     
Flag Coverage Δ
unittests 44.68% <12.50%> (-3.25%) ⬇️

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

Impacted Files Coverage Δ
...che/pulsar/broker/BookKeeperClientFactoryImpl.java 0.00% <0.00%> (ø)
...er/loadbalance/impl/PulsarResourceDescription.java 11.11% <0.00%> (-1.71%) ⬇️
...ava/org/apache/pulsar/broker/service/Consumer.java 70.81% <100.00%> (+6.22%) ⬆️
...ain/java/org/apache/pulsar/broker/rest/Topics.java 0.00% <0.00%> (-100.00%) ⬇️
.../pulsar/broker/rest/RestMessagePublishContext.java 0.00% <0.00%> (-77.78%) ⬇️
...er/metadata/v2/TransactionBufferSnapshotIndex.java 0.00% <0.00%> (-77.78%) ⬇️
.../metadata/v2/TransactionBufferSnapshotSegment.java 0.00% <0.00%> (-75.00%) ⬇️
.../metadata/v2/TransactionBufferSnapshotIndexes.java 0.00% <0.00%> (-71.43%) ⬇️
...ourcegroup/ResourceUsageTopicTransportManager.java 0.00% <0.00%> (-67.83%) ⬇️
...ker/resourcegroup/ResourceQuotaCalculatorImpl.java 4.16% <0.00%> (-62.50%) ⬇️
... and 135 more

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Dec 10, 2022
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

Merging...

@tisonkun tisonkun merged commit 9521872 into apache:master Dec 10, 2022
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 26, 2022
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 29, 2022
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
}

private void incrementUnackedMessages(int ackedMessages) {
private void incrementUnackedMessages(int unackedMessages) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but we also call this with ackedMessages as a negative counter. Yep - very confusing. Very

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs lifecycle/stale Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GetMessageById add RedeliveryCount for output

8 participants