-
Notifications
You must be signed in to change notification settings - Fork 448
MetricBucket reset #521
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
MetricBucket reset #521
Conversation
Could you please reformat your code with |
@binbin0325 Is there something else wrong with my code now? Could you give me some suggestions for improvement or approve the review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@Alipebt Please merge submission records into one |
Signed-off-by: Alipebt <[email protected]>
@binbin0325 I have completed the merge of commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for contributing! |
Signed-off-by: Alipebt <[email protected]>
Signed-off-by: Alipebt <[email protected]>
Describe what this PR does / why we need it
the existing reset() function is not being used and a new MetricBucket is recreated in the ResetBucketTo() function. Therefore, we want to optimize our code to call reset() to avoid this unnecessary operation
Does this pull request fix one issue?
Fixes #403
Describe how you did it
add reset() to ResetBucketTo()
The addRT() mentioned in the issue is not thread-safe, I think we can lose some precision to ensure the efficient execution of the program, so I did not modify the function