[dubbo-go] branch develop updated: Add comments for ServiceMethodLimiter

[fl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

flycash pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git


The following commit(s) were added to refs/heads/develop by this push:
     new 58dd5bd  Add comments for ServiceMethodLimiter
     new 42dbf0d  Merge pull request #490 from flycash/doc-comments
58dd5bd is described below

commit 58dd5bdc4ee99bf1b1191562f0e4a6133ed87603
Author: flycash <fl...@apache.org>
AuthorDate: Thu Apr 23 21:53:15 2020 +0800

    Add comments for ServiceMethodLimiter
---
 .../filter_impl/tps/tps_limiter_method_service.go  | 25 ++++++++++++++++++++--
 filter/tps_limiter.go                              |  1 +
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/filter/filter_impl/tps/tps_limiter_method_service.go b/filter/filter_impl/tps/tps_limiter_method_service.go
index 7fe8de9..2d44c68 100644
--- a/filter/filter_impl/tps/tps_limiter_method_service.go
+++ b/filter/filter_impl/tps/tps_limiter_method_service.go
@@ -115,7 +115,12 @@ type MethodServiceTpsLimiterImpl struct {
 	tpsState *concurrent.Map
 }
 
-// IsAllowable ...
+// IsAllowable based on method-level and service-level.
+// The method-level has high priority which means that if there is any rate limit configuration for the method,
+// the service-level rate limit strategy will be ignored.
+// The key point is how to keep thread-safe
+// This implementation use concurrent map + loadOrStore to make implementation thread-safe
+// You can image that even multiple threads create limiter, but only one could store the limiter into tpsState
 func (limiter MethodServiceTpsLimiterImpl) IsAllowable(url common.URL, invocation protocol.Invocation) bool {
 
 	methodConfigPrefix := "methods." + invocation.MethodName() + "."
@@ -123,23 +128,30 @@ func (limiter MethodServiceTpsLimiterImpl) IsAllowable(url common.URL, invocatio
 	methodLimitRateConfig := url.GetParam(methodConfigPrefix+constant.TPS_LIMIT_RATE_KEY, "")
 	methodIntervalConfig := url.GetParam(methodConfigPrefix+constant.TPS_LIMIT_INTERVAL_KEY, "")
 
+	// service-level tps limit
 	limitTarget := url.ServiceKey()
 
 	// method-level tps limit
 	if len(methodIntervalConfig) > 0 || len(methodLimitRateConfig) > 0 {
+		// it means that if the method-level rate limit exist, we will use method-level rate limit strategy
 		limitTarget = limitTarget + "#" + invocation.MethodName()
 	}
 
+	// looking up the limiter from 'cache'
 	limitState, found := limiter.tpsState.Load(limitTarget)
 	if found {
+		// the limiter has been cached, we return its result
 		return limitState.(filter.TpsLimitStrategy).IsAllowable()
 	}
 
+	// we could not find the limiter, and try to create one.
+
 	limitRate := getLimitConfig(methodLimitRateConfig, url, invocation,
 		constant.TPS_LIMIT_RATE_KEY,
 		constant.DEFAULT_TPS_LIMIT_RATE)
 
 	if limitRate < 0 {
+		// the limitTarget is not necessary to be limited.
 		return true
 	}
 
@@ -150,13 +162,20 @@ func (limiter MethodServiceTpsLimiterImpl) IsAllowable(url common.URL, invocatio
 		panic(fmt.Sprintf("The interval must be positive, please check your configuration! url: %s", url.String()))
 	}
 
+	// find the strategy config and then create one
 	limitStrategyConfig := url.GetParam(methodConfigPrefix+constant.TPS_LIMIT_STRATEGY_KEY,
 		url.GetParam(constant.TPS_LIMIT_STRATEGY_KEY, constant.DEFAULT_KEY))
 	limitStateCreator := extension.GetTpsLimitStrategyCreator(limitStrategyConfig)
+
+	// we using loadOrStore to ensure thread-safe
 	limitState, _ = limiter.tpsState.LoadOrStore(limitTarget, limitStateCreator.Create(int(limitRate), int(limitInterval)))
+
 	return limitState.(filter.TpsLimitStrategy).IsAllowable()
 }
 
+// getLimitConfig will try to fetch the configuration from url.
+// If we can convert the methodLevelConfig to int64, return;
+// Or, we will try to look up server-level configuration and then convert it to int64
 func getLimitConfig(methodLevelConfig string,
 	url common.URL,
 	invocation protocol.Invocation,
@@ -172,6 +191,8 @@ func getLimitConfig(methodLevelConfig string,
 		return result
 	}
 
+	// actually there is no method-level configuration, so we use the service-level configuration
+
 	result, err := strconv.ParseInt(url.GetParam(configKey, defaultVal), 0, 0)
 
 	if err != nil {
@@ -183,7 +204,7 @@ func getLimitConfig(methodLevelConfig string,
 var methodServiceTpsLimiterInstance *MethodServiceTpsLimiterImpl
 var methodServiceTpsLimiterOnce sync.Once
 
-// GetMethodServiceTpsLimiter ...
+// GetMethodServiceTpsLimiter will return an MethodServiceTpsLimiterImpl instance.
 func GetMethodServiceTpsLimiter() filter.TpsLimiter {
 	methodServiceTpsLimiterOnce.Do(func() {
 		methodServiceTpsLimiterInstance = &MethodServiceTpsLimiterImpl{
diff --git a/filter/tps_limiter.go b/filter/tps_limiter.go
index dbc9f76..531eb09 100644
--- a/filter/tps_limiter.go
+++ b/filter/tps_limiter.go
@@ -34,5 +34,6 @@ import (
  *   tps.limiter: "the name of limiter",
  */
 type TpsLimiter interface {
+	// IsAllowable will check whether this invocation should be enabled for further process
 	IsAllowable(common.URL, protocol.Invocation) bool
 }