From 95d38035fd285e6b9addc4f9f62d8f6184489fe8 Mon Sep 17 00:00:00 2001 From: sanxun0325 Date: Mon, 12 Oct 2020 21:38:03 +0800 Subject: [PATCH 1/2] purge existing panic operations --- .../stat/base/atomic_window_wrap_array_test.go | 4 +++- core/stat/base/bucket_leap_array.go | 5 +---- core/stat/base/leap_array.go | 18 +++++++++++++----- core/stat/base/metric_bucket.go | 9 ++++++--- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/core/stat/base/atomic_window_wrap_array_test.go b/core/stat/base/atomic_window_wrap_array_test.go index 7e7b0e702..64a786a63 100644 --- a/core/stat/base/atomic_window_wrap_array_test.go +++ b/core/stat/base/atomic_window_wrap_array_test.go @@ -39,7 +39,9 @@ func Test_atomicBucketWrapArray_elementOffset(t *testing.T) { t.Run(tt.name, func(t *testing.T) { now := uint64(1596199310000) aa := NewAtomicBucketWrapArrayWithTime(tt.args.len, tt.args.bucketLengthInMs, now, tt.args.bg) - if got := uintptr(aa.elementOffset(tt.args.idx)) - uintptr(aa.base); got != tt.want { + offset, ok := aa.elementOffset(tt.args.idx) + assert.True(t, ok) + if got := uintptr(offset) - uintptr(aa.base); got != tt.want { t.Errorf("AtomicBucketWrapArray.elementOffset() = %v, want %v \n", got, tt.want) } }) diff --git a/core/stat/base/bucket_leap_array.go b/core/stat/base/bucket_leap_array.go index 43ae27c1c..309d32cc9 100644 --- a/core/stat/base/bucket_leap_array.go +++ b/core/stat/base/bucket_leap_array.go @@ -1,7 +1,6 @@ package base import ( - "fmt" "sync/atomic" "github.com/alibaba/sentinel-golang/core/base" @@ -30,10 +29,8 @@ func (bla *BucketLeapArray) ResetBucketTo(bw *BucketWrap, startTime uint64) *Buc // sampleCount is the number of slots // intervalInMs is the time length of sliding window +// (intervalInMs%sampleCount != 0,verification has been completed by the caller) func NewBucketLeapArray(sampleCount uint32, intervalInMs uint32) *BucketLeapArray { - if intervalInMs%sampleCount != 0 { - panic(fmt.Sprintf("Invalid parameters, intervalInMs is %d, sampleCount is %d.", intervalInMs, sampleCount)) - } bucketLengthInMs := intervalInMs / sampleCount ret := &BucketLeapArray{ data: LeapArray{ diff --git a/core/stat/base/leap_array.go b/core/stat/base/leap_array.go index 97195af73..ab5dea9b1 100644 --- a/core/stat/base/leap_array.go +++ b/core/stat/base/leap_array.go @@ -7,6 +7,7 @@ import ( "unsafe" "github.com/alibaba/sentinel-golang/core/base" + "github.com/alibaba/sentinel-golang/logging" "github.com/alibaba/sentinel-golang/util" "github.com/pkg/errors" ) @@ -93,25 +94,32 @@ func NewAtomicBucketWrapArray(len int, bucketLengthInMs uint32, generator Bucket return NewAtomicBucketWrapArrayWithTime(len, bucketLengthInMs, util.CurrentTimeMillis(), generator) } -func (aa *AtomicBucketWrapArray) elementOffset(idx int) unsafe.Pointer { +func (aa *AtomicBucketWrapArray) elementOffset(idx int) (unsafe.Pointer, bool) { if idx >= aa.length || idx < 0 { - panic(fmt.Sprintf("The index (%d) is out of bounds, length is %d.", idx, aa.length)) + logging.Error(errors.Errorf("The index (%d) is out of bounds, length is %d.", idx, aa.length), "") + return nil, false } basePtr := aa.base - return unsafe.Pointer(uintptr(basePtr) + uintptr(idx*PtrSize)) + return unsafe.Pointer(uintptr(basePtr) + uintptr(idx*PtrSize)), true } func (aa *AtomicBucketWrapArray) get(idx int) *BucketWrap { // aa.elementOffset(idx) return the secondary pointer of BucketWrap, which is the pointer to the aa.data[idx] // then convert to (*unsafe.Pointer) - return (*BucketWrap)(atomic.LoadPointer((*unsafe.Pointer)(aa.elementOffset(idx)))) + if offset, ok := aa.elementOffset(idx); ok { + return (*BucketWrap)(atomic.LoadPointer((*unsafe.Pointer)(offset))) + } + return nil } func (aa *AtomicBucketWrapArray) compareAndSet(idx int, except, update *BucketWrap) bool { // aa.elementOffset(idx) return the secondary pointer of BucketWrap, which is the pointer to the aa.data[idx] // then convert to (*unsafe.Pointer) // update secondary pointer - return atomic.CompareAndSwapPointer((*unsafe.Pointer)(aa.elementOffset(idx)), unsafe.Pointer(except), unsafe.Pointer(update)) + if offset, ok := aa.elementOffset(idx); ok { + return atomic.CompareAndSwapPointer((*unsafe.Pointer)(offset), unsafe.Pointer(except), unsafe.Pointer(update)) + } + return false } // The BucketWrap leap array, diff --git a/core/stat/base/metric_bucket.go b/core/stat/base/metric_bucket.go index aaacae502..2d330a9ef 100644 --- a/core/stat/base/metric_bucket.go +++ b/core/stat/base/metric_bucket.go @@ -1,10 +1,11 @@ package base import ( - "fmt" "sync/atomic" "github.com/alibaba/sentinel-golang/core/base" + "github.com/alibaba/sentinel-golang/logging" + "github.com/pkg/errors" ) // MetricBucket represents the entity to record metrics per minimum time unit (i.e. the bucket time span). @@ -25,7 +26,8 @@ func NewMetricBucket() *MetricBucket { // Add statistic count for the given metric event. func (mb *MetricBucket) Add(event base.MetricEvent, count int64) { if event >= base.MetricEventTotal || event < 0 { - panic(fmt.Sprintf("Unknown metric event: %v", event)) + logging.Error(errors.Errorf("Unknown metric event: %v", event), "") + return } if event == base.MetricEventRt { mb.AddRt(count) @@ -41,7 +43,8 @@ func (mb *MetricBucket) addCount(event base.MetricEvent, count int64) { // Get current statistic count of the given metric event. func (mb *MetricBucket) Get(event base.MetricEvent) int64 { if event >= base.MetricEventTotal || event < 0 { - panic(fmt.Sprintf("Unknown metric event: %v", event)) + logging.Error(errors.Errorf("Unknown metric event: %v", event), "") + return 0 } return atomic.LoadInt64(&mb.counter[event]) } From 43690edd0da91e6ea01bff0e4655e43f6df89693 Mon Sep 17 00:00:00 2001 From: louyuting <1849491904@qq.com> Date: Wed, 14 Oct 2020 13:32:49 +0800 Subject: [PATCH 2/2] refine --- core/stat/base/bucket_leap_array.go | 3 ++- core/stat/base/leap_array.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/core/stat/base/bucket_leap_array.go b/core/stat/base/bucket_leap_array.go index 309d32cc9..e51b816fb 100644 --- a/core/stat/base/bucket_leap_array.go +++ b/core/stat/base/bucket_leap_array.go @@ -29,7 +29,8 @@ func (bla *BucketLeapArray) ResetBucketTo(bw *BucketWrap, startTime uint64) *Buc // sampleCount is the number of slots // intervalInMs is the time length of sliding window -// (intervalInMs%sampleCount != 0,verification has been completed by the caller) +// sampleCount and intervalInMs must be positive and intervalInMs%sampleCount == 0, +// the validation must be done before call NewBucketLeapArray func NewBucketLeapArray(sampleCount uint32, intervalInMs uint32) *BucketLeapArray { bucketLengthInMs := intervalInMs / sampleCount ret := &BucketLeapArray{ diff --git a/core/stat/base/leap_array.go b/core/stat/base/leap_array.go index ab5dea9b1..79aea788a 100644 --- a/core/stat/base/leap_array.go +++ b/core/stat/base/leap_array.go @@ -96,7 +96,9 @@ func NewAtomicBucketWrapArray(len int, bucketLengthInMs uint32, generator Bucket func (aa *AtomicBucketWrapArray) elementOffset(idx int) (unsafe.Pointer, bool) { if idx >= aa.length || idx < 0 { - logging.Error(errors.Errorf("The index (%d) is out of bounds, length is %d.", idx, aa.length), "") + logging.Error(errors.New("array index out of bounds"), + "array index out of bounds in AtomicBucketWrapArray.elementOffset()", + "idx", idx, "arrayLength", aa.length) return nil, false } basePtr := aa.base