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

[Broker] Fix precision issue and initial value for Consumer#avgMessagesPerEntry #14666

Merged
merged 1 commit into from
Mar 16, 2022
Merged

[Broker] Fix precision issue and initial value for Consumer#avgMessagesPerEntry #14666

merged 1 commit into from
Mar 16, 2022
+19 −14

Conversation

Jason918
Copy link
Contributor

@Jason918 Jason918 commented Mar 12, 2022 •
edited
Loading

Motivation

  1. Precision issue
    There is precision issue to use int type for Consumer#avgMessagesPerEntry.
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());

For example, if tmpAvgMessagesPerEntry = 1 and new value of totalMessages / entries.size() is always 5, then the tmpAvgMessagesPerEntry is always 1 and never increase.

  1. Initial value issue.
    And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

Modifications

  1. Change type of avgMessagesPerEntry to double.
  2. Change init value from 1000 to first totalMessages / entries.size().

Verifying this change

  • Make sure that the change passes the CI checks.

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

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

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

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc
    Internal fix.

Sorry, something went wrong.

@Jason918
fix percision issue for Consumer#avgMessagesPerEntry

Verified

This commit was signed with the committer’s verified signature.
KyleFromNVIDIA Kyle Edwards
SSH Key Fingerprint: HCOKmVHDGRyoBbxy3oHe2oM/H/VI74oAiMzA3TKFtmM
Verified
Learn about vigilant mode
7e41fbb
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 12, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone Mar 15, 2022
@codelipenghui codelipenghui merged commit de2e6c8 into apache:master Mar 16, 2022
codelipenghui pushed a commit that referenced this pull request Mar 18, 2022
@Jason918 @codelipenghui
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
2cb1681 
…esPerEntry (#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double.
2. Change init value from 1000 to first `totalMessages / entries.size()`.

(cherry picked from commit de2e6c8)
@codelipenghui codelipenghui added cherry-picked/branch-2.9 Archived: 2.9 is end of life and removed release/2.10.1 labels Mar 18, 2022
@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.10.0 Mar 19, 2022
codelipenghui pushed a commit that referenced this pull request Mar 19, 2022
@Jason918 @codelipenghui
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
35b2565 
…esPerEntry (#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double.
2. Change init value from 1000 to first `totalMessages / entries.size()`.

(cherry picked from commit de2e6c8)
aparajita89 pushed a commit to aparajita89/pulsar that referenced this pull request Mar 21, 2022
@Jason918
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
04c6619 
…esPerEntry (apache#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double. 
2. Change init value from 1000 to first `totalMessages / entries.size()`.
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
@Jason918
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
1917bd2 
…esPerEntry (apache#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double. 
2. Change init value from 1000 to first `totalMessages / entries.size()`.
Shawyeok pushed a commit to Shawyeok/pulsar that referenced this pull request Sep 5, 2022
@Jason918 @Shawyeok
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
a898c06 
…esPerEntry (apache#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double.
2. Change init value from 1000 to first `totalMessages / entries.size()`.

(cherry picked from commit de2e6c8)
Shawyeok pushed a commit to Shawyeok/pulsar that referenced this pull request Sep 6, 2022
@Jason918 @Shawyeok
[Broker] Fix precision issue and initial value for Consumer#avgMessag…
4e0cc09 
…esPerEntry (apache#14666)

### Motivation

1. Precision issue
There is precision issue to use int type for `Consumer#avgMessagesPerEntry`.
```
tmpAvgMessagesPerEntry = (int) Math.floor(tmpAvgMessagesPerEntry * avgPercent
                + (1 - avgPercent) * totalMessages / entries.size());
```

For example, if `tmpAvgMessagesPerEntry` = 1 and new value of  `totalMessages / entries.size()` is always 5,  then the `tmpAvgMessagesPerEntry` is always 1 and never increase.

2.  Initial value issue.
And the init value of 1000 seems confusing in consumerStats for users, and it need quite a long time to decrease if message rate is very slow.

### Modifications

1. Change type of avgMessagesPerEntry to double.
2. Change init value from 1000 to first `totalMessages / entries.size()`.

(cherry picked from commit de2e6c8)
@zymap zymap mentioned this pull request Sep 15, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui

@aloyszhang aloyszhang

@hangc0276 hangc0276

@eolivelli eolivelli

@315157973 315157973

@rdhabalia rdhabalia

Assignees

@Jason918 Jason918

Labels
cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.9.3
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Jason918 @codelipenghui @aloyszhang