Skip to content

[Broker] Need incrementUnackedMessages when send message to consumer(all subscribe mode)#11998

Closed
shibd wants to merge 1 commit intoapache:masterfrom
shibd:feat-11979
Closed

[Broker] Need incrementUnackedMessages when send message to consumer(all subscribe mode)#11998
shibd wants to merge 1 commit intoapache:masterfrom
shibd:feat-11979

Conversation

@shibd
Copy link
Member

@shibd shibd commented Sep 10, 2021

Motivation

#11979

Modifications

  • Need incrementUnackedMessages when send message to consumer(all subscribe mode)

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

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

Need to update docs?

  • doc-required
  • no-need-doc
  • doc

@shibd
Copy link
Member Author

shibd commented Sep 10, 2021

@jerrypeng Can you help me review it?

Copy link
Contributor

@MarvinCai MarvinCai left a comment

Choose a reason for hiding this comment

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

@shibd
Copy link
Member Author

shibd commented Sep 13, 2021

Looks good, can we add/update existing unit test to verify it? Probably at here: https://github.com/apache/pulsar/blob/master/pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/ConsumerStatsTest.java

@MarvinCai Thank you reply. This PR Maybe it's not right, It will change the original design. I'll close the PR, I will discuss this in issue.

@shibd shibd closed this Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments