Skip to content

Commit

Permalink
fix tps filter panic bug(pre-check the configuration of tps at the st… (
Browse files Browse the repository at this point in the history
apache#1604)

* fix tps filter panic bug(pre-check the configuration of tps at the start phase)

* fix tps limit unit test

* add exists flag to tps filter extension

Co-authored-by: dongjianhui03 <[email protected]>
  • Loading branch information
Mulavar and dongjianhui03 committed Feb 13, 2022
1 parent b087b84 commit 8aea261
Show file tree
Hide file tree
Showing 8 changed files with 221 additions and 121 deletions.
4 changes: 2 additions & 2 deletions common/constant/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ const (
TPSLimiterKey = "tps.limiter"
TPSRejectedExecutionHandlerKey = "tps.limit.rejected.handler"
TPSLimitRateKey = "tps.limit.rate"
DefaultTPSLimitRate = "-1"
DefaultTPSLimitRate = -1
TPSLimitIntervalKey = "tps.limit.interval"
DefaultTPSLimitInterval = "60000"
DefaultTPSLimitInterval = -1
TPSLimitStrategyKey = "tps.limit.strategy"
ExecuteLimitKey = "execute.limit"
DefaultExecuteLimit = "-1"
Expand Down
16 changes: 10 additions & 6 deletions common/extension/tps_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@

package extension

import (
"errors"
)

import (
"dubbo.apache.org/dubbo-go/v3/filter"
)
Expand All @@ -32,13 +36,13 @@ func SetTpsLimiter(name string, creator func() filter.TpsLimiter) {
}

// GetTpsLimiter finds the TpsLimiter with @name
func GetTpsLimiter(name string) filter.TpsLimiter {
func GetTpsLimiter(name string) (filter.TpsLimiter, error) {
creator, ok := tpsLimiter[name]
if !ok {
panic("TpsLimiter for " + name + " is not existing, make sure you have import the package " +
return nil, errors.New("TpsLimiter for " + name + " is not existing, make sure you have import the package " +
"and you have register it by invoking extension.SetTpsLimiter.")
}
return creator()
return creator(), nil
}

// SetTpsLimitStrategy sets the TpsLimitStrategyCreator with @name
Expand All @@ -47,11 +51,11 @@ func SetTpsLimitStrategy(name string, creator filter.TpsLimitStrategyCreator) {
}

// GetTpsLimitStrategyCreator finds the TpsLimitStrategyCreator with @name
func GetTpsLimitStrategyCreator(name string) filter.TpsLimitStrategyCreator {
func GetTpsLimitStrategyCreator(name string) (filter.TpsLimitStrategyCreator, error) {
creator, ok := tpsLimitStrategy[name]
if !ok {
panic("TpsLimitStrategy for " + name + " is not existing, make sure you have import the package " +
return nil, errors.New("TpsLimitStrategy for " + name + " is not existing, make sure you have import the package " +
"and you have register it by invoking extension.SetTpsLimitStrategy.")
}
return creator
return creator, nil
}
2 changes: 1 addition & 1 deletion config/graceful_shutdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func waitForSendingAndReceivingRequests() {
// ignore this step
return
}
rootConfig.Shutdown.RejectRequest = true
rootConfig.Shutdown.RejectRequest.Store(true)
waitingConsumerProcessedTimeout(rootConfig.Shutdown)
}

Expand Down
52 changes: 43 additions & 9 deletions config/method_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@

package config

import (
"fmt"
"strconv"
)

import (
"github.com/creasty/defaults"
)

import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/common/extension"
)

// MethodConfig defines method config
Expand All @@ -43,16 +49,16 @@ type MethodConfig struct {
}

// nolint
func (mc *MethodConfig) Prefix() string {
if len(mc.InterfaceId) != 0 {
return constant.Dubbo + "." + mc.InterfaceName + "." + mc.InterfaceId + "." + mc.Name + "."
func (m *MethodConfig) Prefix() string {
if len(m.InterfaceId) != 0 {
return constant.Dubbo + "." + m.InterfaceName + "." + m.InterfaceId + "." + m.Name + "."
}

return constant.Dubbo + "." + mc.InterfaceName + "." + mc.Name + "."
return constant.Dubbo + "." + m.InterfaceName + "." + m.Name + "."
}

func (mc *MethodConfig) Init() error {
return mc.check()
func (m *MethodConfig) Init() error {
return m.check()
}

func initProviderMethodConfig(sc *ServiceConfig) error {
Expand All @@ -70,9 +76,37 @@ func initProviderMethodConfig(sc *ServiceConfig) error {
}

// check set default value and verify
func (mc *MethodConfig) check() error {
if err := defaults.Set(mc); err != nil {
func (m *MethodConfig) check() error {
qualifieldMethodName := m.InterfaceName + "#" + m.Name
if m.TpsLimitStrategy != "" {
_, err := extension.GetTpsLimitStrategyCreator(m.TpsLimitStrategy)
if err != nil {
panic(err)
}
}

if m.TpsLimitInterval != "" {
tpsLimitInterval, err := strconv.ParseInt(m.TpsLimitInterval, 0, 0)
if err != nil {
return fmt.Errorf("[MethodConfig] Cannot parse the configuration tps.limit.interval for method %s, please check your configuration", qualifieldMethodName)
}
if tpsLimitInterval < 0 {
return fmt.Errorf("[MethodConfig] The configuration tps.limit.interval for %s must be positive, please check your configuration", qualifieldMethodName)
}
}

if m.TpsLimitRate != "" {
tpsLimitRate, err := strconv.ParseInt(m.TpsLimitRate, 0, 0)
if err != nil {
return fmt.Errorf("[MethodConfig] Cannot parse the configuration tps.limit.rate for method %s, please check your configuration", qualifieldMethodName)
}
if tpsLimitRate < 0 {
return fmt.Errorf("[MethodConfig] The configuration tps.limit.rate for method %s must be positive, please check your configuration", qualifieldMethodName)
}
}

if err := defaults.Set(m); err != nil {
return err
}
return verify(mc)
return verify(m)
}
Loading

0 comments on commit 8aea261

Please sign in to comment.