Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix:registry timeout not pars #1392

Merged
merged 5 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions common/constant/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ const (
REGISTRY_PROTOCOL = "registry"
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
// Deprecated use CONFIG_TIMEOUT_KEY key
REGISTRY_TIMEOUT_KEY = "registry.timeout"
REGISTRY_LABEL_KEY = "label"
PREFERRED_KEY = "preferred"
Expand Down
10 changes: 10 additions & 0 deletions common/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strconv"
"strings"
"sync"
"time"
)

import (
Expand Down Expand Up @@ -860,3 +861,12 @@ func SetCompareURLEqualFunc(f CompareURLEqualFunc) {
func GetCompareURLEqualFunc() CompareURLEqualFunc {
return compareURLEqualFunc
}

//GetParamDuration get duration if param is invalid or missing will return 3s
func (c *URL) GetParamDuration(s string, d string) time.Duration {

if t, err := time.ParseDuration(c.GetParam(s, d)); err == nil {
return t
}
return 3 * time.Second
}
2 changes: 1 addition & 1 deletion config/registry_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (c *RegistryConfig) getUrlMap(roleType common.RoleType) url.Values {
urlMap.Set(constant.GROUP_KEY, c.Group)
urlMap.Set(constant.ROLE_KEY, strconv.Itoa(int(roleType)))
urlMap.Set(constant.REGISTRY_KEY, c.Protocol)
urlMap.Set(constant.REGISTRY_TIMEOUT_KEY, c.TimeoutStr)
urlMap.Set(constant.CONFIG_TIMEOUT_KEY, c.TimeoutStr)
// multi registry invoker weight label for load balance
urlMap.Set(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, strconv.FormatBool(true))
urlMap.Set(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, strconv.FormatBool(c.Preferred))
Expand Down
7 changes: 2 additions & 5 deletions config/remote_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
import (
"dubbo.apache.org/dubbo-go/v3/common"
"dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/common/logger"
)

// RemoteConfig: usually we need some middleware, including nacos, zookeeper
Expand All @@ -45,8 +44,8 @@ type RemoteConfig struct {
Params map[string]string `yaml:"params" json:"params,omitempty"`
}

// Prefix
func (c *RemoteConfig) Prefix() string {
// Prefix dubbo.remote.
func (rc *RemoteConfig) Prefix() string {
return constant.RemotePrefix
}

Expand All @@ -56,8 +55,6 @@ func (rc *RemoteConfig) Timeout() time.Duration {
if res, err := time.ParseDuration(rc.TimeoutStr); err == nil {
return res
}
logger.Errorf("Could not parse the timeout string to Duration: %s, the default value will be returned",
rc.TimeoutStr)
return 5 * time.Second
}

Expand Down
3 changes: 1 addition & 2 deletions metadata/report/etcd/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package etcd
import (
"encoding/json"
"strings"
"time"
)

import (
Expand Down Expand Up @@ -146,7 +145,7 @@ type etcdMetadataReportFactory struct{}

// CreateMetadataReport get the MetadataReport instance of etcd
func (e *etcdMetadataReportFactory) CreateMetadataReport(url *common.URL) report.MetadataReport {
timeout, _ := time.ParseDuration(url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT))
timeout := url.GetParamDuration(constant.CONFIG_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT)
addresses := strings.Split(url.Location, ",")
client, err := gxetcd.NewClient(gxetcd.MetadataETCDV3Client, addresses, timeout, 1)
if err != nil {
Expand Down
8 changes: 1 addition & 7 deletions registry/etcdv3/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"path"
"strings"
"sync"
"time"
)

import (
Expand Down Expand Up @@ -75,12 +74,7 @@ func (r *etcdV3Registry) ClientLock() *sync.Mutex {
}

func newETCDV3Registry(url *common.URL) (registry.Registry, error) {
timeout, err := time.ParseDuration(url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT))
if err != nil {
logger.Errorf("timeout config %v is invalid ,err is %v",
url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT), err.Error())
return nil, perrors.WithMessagef(err, "new etcd registry(address:%+v)", url.Location)
}
timeout := url.GetParamDuration(constant.CONFIG_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT)

logger.Infof("etcd address is: %v, timeout is: %s", url.Location, timeout.String())

Expand Down
2 changes: 1 addition & 1 deletion registry/zookeeper/service_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func newZookeeperServiceDiscovery(name string) (registry.ServiceDiscovery, error
common.WithParams(make(url.Values)),
common.WithPassword(remoteConfig.Password),
common.WithUsername(remoteConfig.Username),
common.WithParamsValue(constant.REGISTRY_TIMEOUT_KEY, remoteConfig.TimeoutStr))
common.WithParamsValue(constant.CONFIG_TIMEOUT_KEY, remoteConfig.TimeoutStr))
url.Location = remoteConfig.Address
zksd := &zookeeperServiceDiscovery{
url: url,
Expand Down
5 changes: 1 addition & 4 deletions remoting/nacos/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ func GetNacosConfig(url *common.URL) ([]nacosConstant.ServerConfig, nacosConstan
serverConfigs = append(serverConfigs, nacosConstant.ServerConfig{IpAddr: ip, Port: uint64(port)})
}

timeout, err := time.ParseDuration(url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT))
if err != nil {
return []nacosConstant.ServerConfig{}, nacosConstant.ClientConfig{}, err
}
timeout := url.GetParamDuration(constant.CONFIG_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT)

clientConfig := nacosConstant.ClientConfig{
TimeoutMs: uint64(int32(timeout / time.Millisecond)),
Expand Down
45 changes: 42 additions & 3 deletions remoting/nacos/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ package nacos

import (
"net/url"
"strconv"
"testing"
"time"
)

import (
Expand Down Expand Up @@ -83,15 +83,54 @@ func TestNewNacosClientByUrl(t *testing.T) {
assert.NotNil(t, client)
}

func TestTimeoutConfig(t *testing.T) {
regurlMap := url.Values{}
regurlMap.Set(constant.NACOS_NOT_LOAD_LOCAL_CACHE, "true")
// regurlMap.Set(constant.NACOS_USERNAME, "nacos")
// regurlMap.Set(constant.NACOS_PASSWORD, "nacos")
regurlMap.Set(constant.NACOS_NAMESPACE_ID, "nacos")

t.Run("default timeout", func(t *testing.T) {
newURL, _ := common.NewURL("registry://console.nacos.io:80", common.WithParams(regurlMap))

_, cc, err := GetNacosConfig(newURL)
assert.Nil(t, err)

assert.Equal(t, cc.TimeoutMs, uint64(int32(10*time.Second/time.Millisecond)))
})

t.Run("right timeout", func(t *testing.T) {

regurlMap.Set(constant.CONFIG_TIMEOUT_KEY, "5s")

newURL, _ := common.NewURL("registry://console.nacos.io:80", common.WithParams(regurlMap))

_, cc, err := GetNacosConfig(newURL)
assert.Nil(t, err)

assert.Equal(t, cc.TimeoutMs, uint64(int32(5*time.Second/time.Millisecond)))
})

t.Run("invalid timeout", func(t *testing.T) {
regurlMap.Set(constant.CONFIG_TIMEOUT_KEY, "5ab")

newURL, _ := common.NewURL("registry://console.nacos.io:80", common.WithParams(regurlMap))
_, cc, err := GetNacosConfig(newURL)
assert.Nil(t, err)

assert.Equal(t, cc.TimeoutMs, uint64(int32(3*time.Second/time.Millisecond)))
})

}

func getRegUrl() *common.URL {

regurlMap := url.Values{}
regurlMap.Set(constant.ROLE_KEY, strconv.Itoa(common.PROVIDER))
regurlMap.Set(constant.NACOS_NOT_LOAD_LOCAL_CACHE, "true")
// regurlMap.Set(constant.NACOS_USERNAME, "nacos")
// regurlMap.Set(constant.NACOS_PASSWORD, "nacos")
regurlMap.Set(constant.NACOS_NAMESPACE_ID, "nacos")
regurlMap.Set(constant.REGISTRY_TIMEOUT_KEY, "5s")
regurlMap.Set(constant.CONFIG_TIMEOUT_KEY, "5s")

regurl, _ := common.NewURL("registry://console.nacos.io:80", common.WithParams(regurlMap))

Expand Down
9 changes: 2 additions & 7 deletions remoting/zookeeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package zookeeper

import (
"strings"
"time"
)

import (
Expand Down Expand Up @@ -55,12 +54,8 @@ func ValidateZookeeperClient(container ZkClientFacade, zkName string) error {

if container.ZkClient() == nil {
// in dubbo, every registry only connect one node, so this is []string{r.Address}
timeout, paramErr := time.ParseDuration(url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT))
if paramErr != nil {
logger.Errorf("timeout config %v is invalid, err is %v",
url.GetParam(constant.REGISTRY_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT), paramErr.Error())
return perrors.WithMessagef(paramErr, "newZookeeperClient(address:%+v)", url.Location)
}
timeout := url.GetParamDuration(constant.CONFIG_TIMEOUT_KEY, constant.DEFAULT_REG_TIMEOUT)

zkAddresses := strings.Split(url.Location, ",")
newClient, cltErr := gxzookeeper.NewZookeeperClient(zkName, zkAddresses, true, gxzookeeper.WithZkTimeOut(timeout))
if cltErr != nil {
Expand Down