Improve go docs for core statistic structures and interfaces

Signed-off-by: Eric Zhao <[email protected]>

Author: sczyh30

core/base/stat.go

@@ -81,6 +81,7 @@ func (rs *nopReadStat) AvgRT() float64 {
 }
 
 type WriteStat interface {
+	// AddCount adds given count to the metric of provided MetricEvent.
 	AddCount(event MetricEvent, count int64)
 }
 
@@ -94,6 +95,7 @@ type nopWriteStat struct {
 func (ws *nopWriteStat) AddCount(_ MetricEvent, _ int64) {
 }
 
+// ConcurrencyStat provides read/update operation for concurrency statistics.
 type ConcurrencyStat interface {
 	CurrentConcurrency() int32
 	IncreaseConcurrency()
@@ -126,10 +128,13 @@ func CheckValidityForStatistic(sampleCount, intervalInMs uint32) error {
 	return nil
 }
 
-// CheckValidityForReuseStatistic check the compliance whether readonly metric statistic can be built based on resource's global statistic
-// The parameters, sampleCount and intervalInMs, are the parameters of the metric statistic you want to build
-// The parameters, parentSampleCount and parentIntervalInMs, are the parameters of the resource's global statistic
-// If compliance passes, return nil, if not returns specific error
+// CheckValidityForReuseStatistic checks whether the read-only stat-metric with given attributes
+// (i.e. sampleCount and intervalInMs) can be built based on underlying global statistics data-structure
+// with given attributes (parentSampleCount and parentIntervalInMs). Returns nil if the attributes
+// satisfy the validation, or return specific error if not.
+//
+// The parameters, sampleCount and intervalInMs, are the attributes of the stat-metric view you want to build.
+// The parameters, parentSampleCount and parentIntervalInMs, are the attributes of the underlying statistics data-structure.
 func CheckValidityForReuseStatistic(sampleCount, intervalInMs uint32, parentSampleCount, parentIntervalInMs uint32) error {
 	if intervalInMs == 0 || sampleCount == 0 || intervalInMs%sampleCount != 0 {
 		return IllegalStatisticParamsError
@@ -141,11 +146,11 @@ func CheckValidityForReuseStatistic(sampleCount, intervalInMs uint32, parentSamp
 	}
 	parentBucketLengthInMs := parentIntervalInMs / parentSampleCount
 
-	//SlidingWindowMetric's intervalInMs is not divisible by BucketLeapArray's intervalInMs
+	// intervalInMs of the SlidingWindowMetric is not divisible by BucketLeapArray's intervalInMs
 	if parentIntervalInMs%intervalInMs != 0 {
 		return GlobalStatisticNonReusableError
 	}
-	// BucketLeapArray's BucketLengthInMs is not divisible by SlidingWindowMetric's BucketLengthInMs
+	// BucketLeapArray's BucketLengthInMs is not divisible by BucketLengthInMs of SlidingWindowMetric
 	if bucketLengthInMs%parentBucketLengthInMs != 0 {
 		return GlobalStatisticNonReusableError
 	}

core/stat/base/bucket_leap_array.go

@@ -24,7 +24,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-// The implementation of sliding window based on LeapArray (as the sliding window infrastructure)
+// BucketLeapArray is the sliding window implementation based on LeapArray (as the sliding window infrastructure)
 // and MetricBucket (as the data type). The MetricBucket is used to record statistic
 // metrics per minimum time unit (i.e. the bucket time span).
 type BucketLeapArray struct {
@@ -42,11 +42,14 @@ func (bla *BucketLeapArray) ResetBucketTo(bw *BucketWrap, startTime uint64) *Buc
 	return bw
 }
 
-// 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
+// NewBucketLeapArray creates a BucketLeapArray with given attributes.
+//
+// The sampleCount represents the number of buckets, while intervalInMs represents
+// the total time span of sliding window. Note that the sampleCount and intervalInMs must be positive
+// and satisfies the condition that intervalInMs%sampleCount == 0.
+// The validation must be done before call NewBucketLeapArray.
 func NewBucketLeapArray(sampleCount uint32, intervalInMs uint32) *BucketLeapArray {
+	// TODO: also check params here.
 	bucketLengthInMs := intervalInMs / sampleCount
 	ret := &BucketLeapArray{
 		data: LeapArray{
@@ -82,9 +85,8 @@ func (bla *BucketLeapArray) GetIntervalInSecond() float64 {
 	return float64(bla.IntervalInMs()) / 1000.0
 }
 
-// Write method
-// It might panic
 func (bla *BucketLeapArray) AddCount(event base.MetricEvent, count int64) {
+	// It might panic?
 	bla.addCountWithTime(util.CurrentTimeMillis(), event, count)
 }
 
@@ -131,9 +133,9 @@ func (bla *BucketLeapArray) currentBucketWithTime(now uint64) *MetricBucket {
 	return b
 }
 
-// Read method, need to adapt upper application
-// it might panic
+// Count returns the sum count for the given MetricEvent within all valid (non-expired) buckets.
 func (bla *BucketLeapArray) Count(event base.MetricEvent) int64 {
+	// it might panic?
 	return bla.CountWithTime(util.CurrentTimeMillis(), event)
 }
 
@@ -159,12 +161,14 @@ func (bla *BucketLeapArray) CountWithTime(now uint64, event base.MetricEvent) in
 	return count
 }
 
-// Read method, get all BucketWrap.
+// Values returns all valid (non-expired) buckets.
 func (bla *BucketLeapArray) Values(now uint64) []*BucketWrap {
+	// Refresh current bucket if necessary.
 	_, err := bla.data.currentBucketOfTime(now, bla)
 	if err != nil {
-		logging.Error(err, "Failed to get current bucket in BucketLeapArray.Values()", "now", now)
+		logging.Error(err, "Failed to refresh current bucket in BucketLeapArray.Values()", "now", now)
 	}
+
 	return bla.data.valuesWithTime(now)
 }
 

core/stat/base/leap_array.go

@@ -26,15 +26,16 @@ import (
 	"github.com/pkg/errors"
 )
 
-// BucketWrap represent a slot to record metrics
-// In order to reduce the usage of memory, BucketWrap don't hold length of BucketWrap
+// BucketWrap represents a slot to record metrics.
+//
+// In order to reduce memory footprint, BucketWrap does not hold the length of the bucket.
 // The length of BucketWrap could be seen in LeapArray.
-// The scope of time is [startTime, startTime+bucketLength)
-// The size of BucketWrap is 24(8+16) bytes
+// The scope of time is [startTime, startTime+bucketLength).
+// The size of BucketWrap is 24(8+16) bytes.
 type BucketWrap struct {
-	// The start timestamp of this statistic bucket wrapper.
+	// BucketStart represents start timestamp of this statistic bucket wrapper.
 	BucketStart uint64
-	// The actual data structure to record the metrics (e.g. MetricBucket).
+	// Value represents the actual data structure of the metrics (e.g. MetricBucket).
 	Value atomic.Value
 }
 
@@ -50,8 +51,9 @@ func calculateStartTime(now uint64, bucketLengthInMs uint32) uint64 {
 	return now - (now % uint64(bucketLengthInMs))
 }
 
-// atomic BucketWrap array to resolve race condition
-// AtomicBucketWrapArray can not append or delete element after initializing
+// AtomicBucketWrapArray represents a thread-safe circular array.
+//
+// The length of the array should be provided on-create and cannot be modified.
 type AtomicBucketWrapArray struct {
 	// The base address for real data array
 	base unsafe.Pointer
@@ -94,11 +96,11 @@ func NewAtomicBucketWrapArrayWithTime(len int, bucketLengthInMs uint32, now uint
 	return ret
 }
 
-// New AtomicBucketWrapArray with initializing field data
-// Default, automatically initialize each BucketWrap
-// len: length of array
-// bucketLengthInMs: bucket length of BucketWrap
-// generator: generator to generate bucket
+// NewAtomicBucketWrapArray creates an AtomicBucketWrapArray and initializes data of each BucketWrap.
+//
+// The len represents the length of the circular array.
+// The bucketLengthInMs represents bucket length of each bucket (in milliseconds).
+// The generator accepts a BucketGenerator to generate and refresh buckets.
 func NewAtomicBucketWrapArray(len int, bucketLengthInMs uint32, generator BucketGenerator) *AtomicBucketWrapArray {
 	return NewAtomicBucketWrapArrayWithTime(len, bucketLengthInMs, util.CurrentTimeMillis(), generator)
 }
@@ -133,23 +135,30 @@ func (aa *AtomicBucketWrapArray) compareAndSet(idx int, except, update *BucketWr
 	return false
 }
 
-// The BucketWrap leap array,
-// sampleCount represent the number of BucketWrap
-// intervalInMs represent the interval of LeapArray.
-// For example, bucketLengthInMs is 200ms, intervalInMs is 1000ms, so sampleCount is 5.
-// Give a diagram to illustrate
-// Suppose current time is 888, bucketLengthInMs is 200ms, intervalInMs is 1000ms, LeapArray will build the below windows
+// LeapArray represents the fundamental implementation of a sliding window data-structure.
+//
+// Some important attributes: the sampleCount represents the number of buckets,
+// while intervalInMs represents the total time span of the sliding window.
+//
+// For example, assuming sampleCount=5, intervalInMs is 1000ms, so the bucketLength is 200ms.
+// Let's give a diagram to illustrate.
+// Suppose current timestamp is 1188, bucketLength is 200ms, intervalInMs is 1000ms, then
+// time span of current bucket is [1000, 1200). The representation of the underlying structure:
+//
 //   B0       B1      B2     B3      B4
 //   |_______|_______|_______|_______|_______|
-//  1000    1200    1400    1600    800    (1000)
-//                                        ^
-//                                      time=888
+//  1000    1200    400     600     800    (1000) ms
+//         ^
+//      time=1188
 type LeapArray struct {
 	bucketLengthInMs uint32
-	sampleCount      uint32
-	intervalInMs     uint32
-	array            *AtomicBucketWrapArray
-	// update lock
+	// sampleCount represents the number of BucketWrap.
+	sampleCount uint32
+	// intervalInMs represents the total time span of the sliding window (in milliseconds).
+	intervalInMs uint32
+	// array represents the internal circular array.
+	array *AtomicBucketWrapArray
+	// updateLock is the internal lock for update operations.
 	updateLock mutex
 }
 
@@ -224,7 +233,8 @@ func (la *LeapArray) calculateTimeIdx(now uint64) int {
 	return int(timeId) % la.array.length
 }
 
-//  Get all BucketWrap between [current time - leap array interval, current time]
+// Values returns all valid (non-expired) buckets between [curBucketEnd-windowInterval, curBucketEnd],
+// where curBucketEnd=curBucketStart+bucketLength.
 func (la *LeapArray) Values() []*BucketWrap {
 	return la.valuesWithTime(util.CurrentTimeMillis())
 }
@@ -244,6 +254,8 @@ func (la *LeapArray) valuesWithTime(now uint64) []*BucketWrap {
 	return ret
 }
 
+// ValuesConditional returns all buckets of which the startTimestamp satisfies the given timestamp condition (predicate).
+// The function uses the parameter "now" as the target timestamp.
 func (la *LeapArray) ValuesConditional(now uint64, predicate base.TimePredicate) []*BucketWrap {
 	if now <= 0 {
 		return make([]*BucketWrap, 0)
@@ -259,17 +271,17 @@ func (la *LeapArray) ValuesConditional(now uint64, predicate base.TimePredicate)
 	return ret
 }
 
-// Judge whether the BucketWrap is expired
+// isBucketDeprecated checks whether the BucketWrap is expired, according to given timestamp.
 func (la *LeapArray) isBucketDeprecated(now uint64, ww *BucketWrap) bool {
 	ws := atomic.LoadUint64(&ww.BucketStart)
 	return (now - ws) > uint64(la.intervalInMs)
 }
 
-// Generic interface to generate bucket
+// BucketGenerator represents the "generic" interface for generating and refreshing buckets.
 type BucketGenerator interface {
-	// called when timestamp entry a new slot interval
+	// NewEmptyBucket creates new raw data inside the bucket.
 	NewEmptyBucket() interface{}
 
-	// reset the BucketWrap, clear all data of BucketWrap
-	ResetBucketTo(bw *BucketWrap, startTime uint64) *BucketWrap
+	// ResetBucketTo refreshes the BucketWrap to provided startTime and resets all data inside the given bucket.
+	ResetBucketTo(bucket *BucketWrap, startTime uint64) *BucketWrap
 }

core/stat/base/sliding_window_metric.go

@@ -25,17 +25,18 @@ import (
 )
 
 // SlidingWindowMetric represents the sliding window metric wrapper.
-// It does not store any data and is the wrapper of BucketLeapArray to adapt to different internal bucket
-// SlidingWindowMetric is used for SentinelRules and BucketLeapArray is used for monitor
-// BucketLeapArray is per resource, and SlidingWindowMetric support only read operation.
+// It does not store any data and is the wrapper of BucketLeapArray to adapt to different internal bucket.
+//
+// SlidingWindowMetric is designed as a high-level, read-only statistic structure for functionalities of Sentinel
 type SlidingWindowMetric struct {
 	bucketLengthInMs uint32
 	sampleCount      uint32
 	intervalInMs     uint32
 	real             *BucketLeapArray
 }
 
-// It must pass the parameter point to the real storage entity
+// NewSlidingWindowMetric creates a SlidingWindowMetric with given attributes.
+// The pointer to the internal statistic BucketLeapArray should be valid.
 func NewSlidingWindowMetric(sampleCount, intervalInMs uint32, real *BucketLeapArray) (*SlidingWindowMetric, error) {
 	if real == nil {
 		return nil, errors.New("nil BucketLeapArray")
@@ -53,7 +54,7 @@ func NewSlidingWindowMetric(sampleCount, intervalInMs uint32, real *BucketLeapAr
 	}, nil
 }
 
-// Get the start time range of the bucket for the provided time.
+// getBucketStartRange returns start time range of the bucket for the provided time.
 // The actual time span is: [start, end + in.bucketTimeLength)
 func (m *SlidingWindowMetric) getBucketStartRange(timeMs uint64) (start, end uint64) {
 	curBucketStartTime := calculateStartTime(timeMs, m.real.BucketLengthInMs())
@@ -107,6 +108,8 @@ func (m *SlidingWindowMetric) getQPSWithTime(now uint64, event base.MetricEvent)
 
 func (m *SlidingWindowMetric) getSatisfiedBuckets(now uint64) []*BucketWrap {
 	start, end := m.getBucketStartRange(now)
+	// Extracts the buckets of which the startTime is between [start, end]
+	// which means the time view of the buckets is [firstStart, endStart+bucketLength)
 	satisfiedBuckets := m.real.ValuesConditional(now, func(ws uint64) bool {
 		return ws >= start && ws <= end
 	})