Skip to content

Conversation

binbin0325
Copy link
Collaborator

Describe what this PR does / why we need it

A panic operation will cause the program to exit

Does this pull request fix one issue?

fixes:#278

Describe how you did it

Describe how to verify it

go test ./...

Special notes for reviews

@louyuting louyuting linked an issue Oct 13, 2020 that may be closed by this pull request
@louyuting louyuting added this to the 1.0.0 milestone Oct 13, 2020
@louyuting louyuting added the kind/enhancement Category issues or PRs related to enhancement label Oct 13, 2020
@binbin0325 binbin0325 force-pushed the purge_existing_panic branch from 6a531e9 to d083f66 Compare October 13, 2020 13:52
@codecov-io
Copy link

codecov-io commented Oct 14, 2020

Codecov Report

Merging #284 into master will increase coverage by 0.07%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   51.09%   51.17%   +0.07%     
==========================================
  Files          79       79              
  Lines        4026     4057      +31     
==========================================
+ Hits         2057     2076      +19     
- Misses       1679     1692      +13     
+ Partials      290      289       -1     
Impacted Files Coverage Δ
core/stat/base/bucket_leap_array.go 59.45% <ø> (+1.56%) ⬆️
core/stat/base/metric_bucket.go 71.42% <0.00%> (-7.52%) ⬇️
util/atomic.go 51.61% <16.66%> (-3.56%) ⬇️
core/stat/base/leap_array.go 71.87% <45.45%> (-3.41%) ⬇️
core/base/slot_chain.go 92.95% <0.00%> (-4.06%) ⬇️
api/api.go 19.44% <0.00%> (-0.85%) ⬇️
core/flow/rule.go 19.04% <0.00%> (ø)
core/isolation/slot.go 0.00% <0.00%> (ø)
core/circuitbreaker/slot.go 100.00% <0.00%> (ø)
core/flow/rule_manager.go 61.36% <0.00%> (+0.29%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb639b4...9c5e1ed. Read the comment docs.

// sampleCount is the number of slots
// intervalInMs is the time length of sliding window
// sampleCount and intervalInMs must be positive and intervalInMs%sampleCount == 0,
// the validation must be done before call NewBucketLeapArray
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about change the param to bucket span time and bucket count in future? Is it more easy to use?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is your meaning bucket count and bucketLengthMs?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could file another PR to enhance here

Copy link
Collaborator

@luckyxiaoqiang luckyxiaoqiang Oct 15, 2020

Choose a reason for hiding this comment

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

I'll have a try, please add an issue and assign to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can submit a issue.

@louyuting louyuting force-pushed the purge_existing_panic branch from 56c1f60 to 43690ed Compare October 15, 2020 13:51
Copy link
Collaborator

@louyuting louyuting left a comment

Choose a reason for hiding this comment

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

Thanks for nice work

@louyuting louyuting merged commit 1555a81 into alibaba:master Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or PRs related to enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Purge existing panic operations
5 participants