You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2022/04/04 02:47:56 UTC
[GitHub] [dubbo-go-pixiu] ztelur opened a new pull request, #389: Bugfix nacos registry bug
ztelur opened a new pull request, #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
- fix nacos registry bug
- delete useless dubbo version code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r846574632
##########
pkg/adapter/dubboregistry/registry/nacos/service_listener.go:
##########
@@ -58,11 +64,76 @@ func newNacosSrvListener(url *dubboCommon.URL, client naming_client.INamingClien
client: client,
exit: make(chan struct{}),
adapterListener: adapterListener,
+ instanceMap: map[string]nacosModel.Instance{},
}
}
-func (z *serviceListener) Notify(e *dubboRegistry.ServiceEvent) {
- bkConfig, methods, location, err := registry.ParseDubboString(e.Service.String())
+func (z *serviceListener) Callback(services []nacosModel.SubscribeService, err error) {
+ if err != nil {
+ logger.Errorf("nacos subscribe callback error:%s", err.Error())
+ return
+ }
+
+ addInstances := make([]nacosModel.Instance, 0, len(services))
+ delInstances := make([]nacosModel.Instance, 0, len(services))
+ updateInstances := make([]nacosModel.Instance, 0, len(services))
+ newInstanceMap := make(map[string]nacosModel.Instance, len(services))
+
+ z.cacheLock.Lock()
Review Comment:
这个锁的粒度太大了，增大出现死锁的概率，且导致整体很低效。下面的函数动作确定都在lock scope 内吗？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] mark4z commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
mark4z commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r841395040
##########
pkg/client/dubbo/dubbo.go:
##########
@@ -323,7 +323,7 @@ func (dc *Client) create(key string, irequest fc.IntegrationRequest) *generic.Ge
// sleep when first call to fetch enough service meta data from nacos
// todo: GenericLoad should guarantee it
if useNacosRegister {
Review Comment:
useNacosRegister Not effective when registerId not equals RegistryType
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r850511951
##########
samples/dubbogo/simple/nacos/server/app/user.go:
##########
@@ -0,0 +1,271 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "sync"
+ "time"
+)
+
+import (
+ "dubbo.apache.org/dubbo-go/v3/config"
+
+ hessian "github.com/apache/dubbo-go-hessian2"
+)
+
+func init() {
+ config.SetProviderService(new(UserProvider))
+ // ------for hessian2------
+ hessian.RegisterPOJO(&User{})
+
+ cache = &UserDB{
+ nameIndex: make(map[string]*User, 16),
+ codeIndex: make(map[int64]*User, 16),
+ lock: sync.Mutex{},
+ }
+
+ cache.Add(&User{ID: "0001", Code: 1, Name: "tc", Age: 18, Time: time.Now()})
+ cache.Add(&User{ID: "0002", Code: 2, Name: "ic", Age: 88, Time: time.Now()})
+}
+
+var cache *UserDB
+
+// UserDB cache user.
+type UserDB struct {
+ // key is name, value is user obj
+ nameIndex map[string]*User
+ // key is code, value is user obj
+ codeIndex map[int64]*User
+ lock sync.Mutex
+}
+
+// nolint
+func (db *UserDB) Add(u *User) bool {
+ if u.Name == "" || u.Code <= 0 {
+ return false
+ }
+
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ if !db.existName(u.Name) && !db.existCode(u.Code) {
+ return db.AddForName(u) && db.AddForCode(u)
+ }
+
+ return false
+}
+
+// nolint
+func (db *UserDB) AddForName(u *User) bool {
+ if len(u.Name) == 0 {
+ return false
+ }
+
+ if _, ok := db.nameIndex[u.Name]; ok {
+ return false
+ }
+
+ db.nameIndex[u.Name] = u
+ return true
+}
+
+// nolint
+func (db *UserDB) AddForCode(u *User) bool {
+ if u.Code <= 0 {
+ return false
+ }
+
+ if _, ok := db.codeIndex[u.Code]; ok {
+ return false
+ }
+
+ db.codeIndex[u.Code] = u
+ return true
+}
+
+// nolint
+func (db *UserDB) GetByName(n string) (*User, bool) {
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ r, ok := db.nameIndex[n]
+ return r, ok
+}
+
+// nolint
+func (db *UserDB) GetByCode(n int64) (*User, bool) {
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ r, ok := db.codeIndex[n]
+ return r, ok
+}
+
+func (db *UserDB) existName(name string) bool {
+ if len(name) <= 0 {
+ return false
+ }
+
+ _, ok := db.nameIndex[name]
+ if ok {
+ return true
+ }
+
+ return false
+}
+
+func (db *UserDB) existCode(code int64) bool {
+ if code <= 0 {
+ return false
+ }
+
+ _, ok := db.codeIndex[code]
+ if ok {
+ return true
+ }
+
+ return false
+}
+
+// User user obj.
+type User struct {
+ ID string `json:"id,omitempty"`
+ Code int64 `json:"code,omitempty"`
+ Name string `json:"name,omitempty"`
+ Age int32 `json:"age,omitempty"`
+ Time time.Time `json:"time,omitempty"`
+}
+
+// UserProvider the dubbo provider.
+// like: version: 1.0.0 group: test
+type UserProvider struct{}
+
+// CreateUser new user, PX config POST.
+func (u *UserProvider) CreateUser(ctx context.Context, user *User) (*User, error) {
+ outLn("Req CreateUser data:%#v", user)
+ if user == nil {
+ return nil, errors.New("not found")
+ }
+ _, ok := cache.GetByName(user.Name)
+ if ok {
+ return nil, errors.New("data is exist")
+ }
+
+ b := cache.Add(user)
+ if b {
+ return user, nil
+ }
+
+ return nil, errors.New("add error")
+}
+
+// GetUserByName query by name, single param, PX config GET.
+func (u *UserProvider) GetUserByName(ctx context.Context, name string) (*User, error) {
+ outLn("Req GetUserByName name:%#v", name)
+ r, ok := cache.GetByName(name)
+ if ok {
+ outLn("Req GetUserByName result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserByCode query by code, single param, PX config GET.
+func (u *UserProvider) GetUserByCode(ctx context.Context, code int64) (*User, error) {
+ outLn("Req GetUserByCode name:%#v", code)
+ r, ok := cache.GetByCode(code)
+ if ok {
+ outLn("Req GetUserByCode result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserTimeout query by name, will timeout for pixiu.
+func (u *UserProvider) GetUserTimeout(ctx context.Context, name string) (*User, error) {
+ outLn("Req GetUserByName name:%#v", name)
+ // sleep 10s, pixiu config less than 10s.
+ time.Sleep(10 * time.Second)
+ r, ok := cache.GetByName(name)
+ if ok {
+ outLn("Req GetUserByName result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserByNameAndAge query by name and age, two params, PX config GET.
+func (u *UserProvider) GetUserByNameAndAge(ctx context.Context, name string, age int32) (*User, error) {
+ outLn("Req GetUserByNameAndAge name:%s, age:%d", name, age)
+ r, ok := cache.GetByName(name)
+ if ok && r.Age == age {
+ outLn("Req GetUserByNameAndAge result:%#v", r)
+ return r, nil
+ }
+ return r, nil
+}
+
+// UpdateUser update by user struct, my be another struct, PX config POST or PUT.
+func (u *UserProvider) UpdateUser(ctx context.Context, user *User) (bool, error) {
+ outLn("Req UpdateUser data:%#v", user)
+ r, ok := cache.GetByName(user.Name)
+ if ok {
+ if user.ID != "" {
+ r.ID = user.ID
+ }
+ if user.Age >= 0 {
+ r.Age = user.Age
+ }
+ return true, nil
+ }
+ return false, errors.New("not found")
+}
+
+// UpdateUserByName update by user struct, my be another struct, PX config POST or PUT.
+func (u *UserProvider) UpdateUserByName(ctx context.Context, name string, user *User) (bool, error) {
+ outLn("Req UpdateUserByName data:%#v", user)
+ r, ok := cache.GetByName(name)
+ if ok {
+ if user.ID != "" {
+ r.ID = user.ID
+ }
+ if user.Age >= 0 {
+ r.Age = user.Age
+ }
+ return true, nil
+ }
+ return false, errors.New("not found")
+}
+
+// nolint
+func (u *UserProvider) Reference() string {
+ return "UserProvider"
+}
+
+// nolint
+func (u User) JavaClassName() string {
+ return "com.dubbogo.pixiu.User"
+}
+
+// nolint
+func outLn(format string, args ...interface{}) {
+ fmt.Printf("\033[32;40m"+format+"\033[0m\n", args...)
Review Comment:
delete outLn function in all samples
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] codecov-commenter commented on pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#issuecomment-1087055017
# [Codecov](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#389](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (936f36c) into [develop](https://codecov.io/gh/apache/dubbo-go-pixiu/commit/6e1411595131fb3daed1b1d5c713b02656286855?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6e14115) will **increase** coverage by `1.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## develop #389 +/- ##
===========================================
+ Coverage 38.32% 39.33% +1.01%
===========================================
Files 59 54 -5
Lines 3987 3465 -522
===========================================
- Hits 1528 1363 -165
+ Misses 2280 1939 -341
+ Partials 179 163 -16
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/client/dubbo/dubbo.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NsaWVudC9kdWJiby9kdWJiby5nbw==) | `19.46% <0.00%> (ø)` | |
| [pkg/server/cluster\_manager.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NlcnZlci9jbHVzdGVyX21hbmFnZXIuZ28=) | `37.06% <0.00%> (-10.71%)` | :arrow_down: |
| [pkg/server/pixiu\_start.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NlcnZlci9waXhpdV9zdGFydC5nbw==) | `0.00% <0.00%> (-3.78%)` | :arrow_down: |
| [pkg/server/dynamic\_resource\_manager.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NlcnZlci9keW5hbWljX3Jlc291cmNlX21hbmFnZXIuZ28=) | `88.88% <0.00%> (-0.59%)` | :arrow_down: |
| [pkg/config/xds/xds.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbmZpZy94ZHMveGRzLmdv) | | |
| [pkg/config/xds/apiclient/grpc.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbmZpZy94ZHMvYXBpY2xpZW50L2dycGMuZ28=) | | |
| [pkg/config/xds/cds.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbmZpZy94ZHMvY2RzLmdv) | | |
| [pkg/config/xds/apiclient/apiclient.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbmZpZy94ZHMvYXBpY2xpZW50L2FwaWNsaWVudC5nbw==) | | |
| [pkg/config/xds/lds.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NvbmZpZy94ZHMvbGRzLmdv) | | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6e14115...936f36c](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/389?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r850512917
##########
pkg/adapter/dubboregistry/registry/nacos/service_listener.go:
##########
@@ -58,11 +64,76 @@ func newNacosSrvListener(url *dubboCommon.URL, client naming_client.INamingClien
client: client,
exit: make(chan struct{}),
adapterListener: adapterListener,
+ instanceMap: map[string]nacosModel.Instance{},
}
}
-func (z *serviceListener) Notify(e *dubboRegistry.ServiceEvent) {
- bkConfig, methods, location, err := registry.ParseDubboString(e.Service.String())
+func (z *serviceListener) Callback(services []nacosModel.SubscribeService, err error) {
+ if err != nil {
+ logger.Errorf("nacos subscribe callback error:%s", err.Error())
+ return
+ }
+
+ addInstances := make([]nacosModel.Instance, 0, len(services))
+ delInstances := make([]nacosModel.Instance, 0, len(services))
+ updateInstances := make([]nacosModel.Instance, 0, len(services))
+ newInstanceMap := make(map[string]nacosModel.Instance, len(services))
+
+ z.cacheLock.Lock()
Review Comment:
有一个 channel 也可以，但是感觉又引入了一层，增加复杂度
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] mark4z commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
mark4z commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r849493701
##########
pkg/adapter/dubboregistry/registry/nacos/service_listener.go:
##########
@@ -58,11 +64,76 @@ func newNacosSrvListener(url *dubboCommon.URL, client naming_client.INamingClien
client: client,
exit: make(chan struct{}),
adapterListener: adapterListener,
+ instanceMap: map[string]nacosModel.Instance{},
}
}
-func (z *serviceListener) Notify(e *dubboRegistry.ServiceEvent) {
- bkConfig, methods, location, err := registry.ParseDubboString(e.Service.String())
+func (z *serviceListener) Callback(services []nacosModel.SubscribeService, err error) {
+ if err != nil {
+ logger.Errorf("nacos subscribe callback error:%s", err.Error())
+ return
+ }
+
+ addInstances := make([]nacosModel.Instance, 0, len(services))
+ delInstances := make([]nacosModel.Instance, 0, len(services))
+ updateInstances := make([]nacosModel.Instance, 0, len(services))
+ newInstanceMap := make(map[string]nacosModel.Instance, len(services))
+
+ z.cacheLock.Lock()
Review Comment:
考虑一下channel？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r841532196
##########
pkg/adapter/dubboregistry/registry/nacos/service_listener.go:
##########
@@ -104,3 +175,49 @@ func (zkl *serviceListener) Close() {
close(zkl.exit)
zkl.wg.Wait()
}
+
+func generateUrl(instance nacosModel.Instance) *dubboCommon.URL {
Review Comment:
generateURL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r841531860
##########
pkg/adapter/dubboregistry/registry/nacos/interface_listener.go:
##########
@@ -146,53 +151,74 @@ func fromServiceFullKey(fullKey string) *serviceInfo {
}
func (z *nacosIntfListener) updateServiceList(serviceList []string) error {
- // todo lock all svc listener
+ // add new service info and watch
+
+ newServiceMap := make(map[string]bool)
- allSvcListener := z.reg.GetAllSvcListener()
- subscribedServiceKeysMap := make(map[string]bool)
- for k := range allSvcListener {
- subscribedServiceKeysMap[k] = true
- }
- serviceNeedUpdate := make([]*serviceInfo, 0)
for _, v := range serviceList {
svcInfo := fromServiceFullKey(v)
if svcInfo == nil {
// invalid nacos dubbo service key
continue
}
key := svcInfo.String()
- if _, ok := allSvcListener[key]; !ok {
- serviceNeedUpdate = append(serviceNeedUpdate, svcInfo)
- } else {
- delete(subscribedServiceKeysMap, key)
+ newServiceMap[key] = true
+ if _, ok := z.serviceInfoMap[key]; !ok {
+
+ url, _ := dubboCommon.NewURL("mock://localhost:8848")
+ url.SetParam(constant.InterfaceKey, svcInfo.interfaceName)
+ url.SetParam(constant.GroupKey, svcInfo.group)
+ url.SetParam(constant.VersionKey, svcInfo.version)
+ l := newNacosSrvListener(url, z.client, z.adapterListener)
+ l.wg.Add(1)
+
+ svcInfo.listener = l
+ z.serviceInfoMap[key] = svcInfo
+
+ go func(v *serviceInfo) {
+ defer l.wg.Done()
Review Comment:
这个 wg 在哪里 wait 啊？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r846949635
##########
samples/dubbogo/simple/nacos/server/app/user.go:
##########
@@ -0,0 +1,271 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "sync"
+ "time"
+)
+
+import (
+ "dubbo.apache.org/dubbo-go/v3/config"
+
+ hessian "github.com/apache/dubbo-go-hessian2"
+)
+
+func init() {
+ config.SetProviderService(new(UserProvider))
+ // ------for hessian2------
+ hessian.RegisterPOJO(&User{})
+
+ cache = &UserDB{
+ nameIndex: make(map[string]*User, 16),
+ codeIndex: make(map[int64]*User, 16),
+ lock: sync.Mutex{},
+ }
+
+ cache.Add(&User{ID: "0001", Code: 1, Name: "tc", Age: 18, Time: time.Now()})
+ cache.Add(&User{ID: "0002", Code: 2, Name: "ic", Age: 88, Time: time.Now()})
+}
+
+var cache *UserDB
+
+// UserDB cache user.
+type UserDB struct {
+ // key is name, value is user obj
+ nameIndex map[string]*User
+ // key is code, value is user obj
+ codeIndex map[int64]*User
+ lock sync.Mutex
+}
+
+// nolint
+func (db *UserDB) Add(u *User) bool {
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ if u.Name == "" || u.Code <= 0 {
Review Comment:
这个判断条件完全可以挪到 lock scope 上面。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] mark4z commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
mark4z commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r841395369
##########
pkg/client/dubbo/dubbo.go:
##########
@@ -323,7 +323,7 @@ func (dc *Client) create(key string, irequest fc.IntegrationRequest) *generic.Ge
// sleep when first call to fetch enough service meta data from nacos
// todo: GenericLoad should guarantee it
if useNacosRegister {
Review Comment:
useNacosRegister := false
registerIds := make([]string, 0)
for k := range dc.rootConfig.Registries {
registerIds = append(registerIds, k)
if k == "nacos" { // k == 'nacos'? it's just a name
useNacosRegister = true
}
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r849258023
##########
pkg/adapter/dubboregistry/registry/nacos/service_listener.go:
##########
@@ -58,11 +64,76 @@ func newNacosSrvListener(url *dubboCommon.URL, client naming_client.INamingClien
client: client,
exit: make(chan struct{}),
adapterListener: adapterListener,
+ instanceMap: map[string]nacosModel.Instance{},
}
}
-func (z *serviceListener) Notify(e *dubboRegistry.ServiceEvent) {
- bkConfig, methods, location, err := registry.ParseDubboString(e.Service.String())
+func (z *serviceListener) Callback(services []nacosModel.SubscribeService, err error) {
+ if err != nil {
+ logger.Errorf("nacos subscribe callback error:%s", err.Error())
+ return
+ }
+
+ addInstances := make([]nacosModel.Instance, 0, len(services))
+ delInstances := make([]nacosModel.Instance, 0, len(services))
+ updateInstances := make([]nacosModel.Instance, 0, len(services))
+ newInstanceMap := make(map[string]nacosModel.Instance, len(services))
+
+ z.cacheLock.Lock()
Review Comment:
- service_listener的 Callback 函数只会处理某个service的instance变更回调，并发理论上不会很高(依赖 nacos_client subscribe 回调的实现机制，可能也不存在并发，都是逐次回调)；
- 下面的逻辑是根据内存缓存中的 instance 元信息和回调的最新元信息一起计算，得出新增或删除的 instance，然后覆盖内存缓存的 instance 元信息，应该加锁，不然理论上可能会出现两次回调同时进行，导致更新覆盖的问题。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r850093973
##########
samples/dubbogo/simple/nacos/server/app/user.go:
##########
@@ -0,0 +1,271 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "sync"
+ "time"
+)
+
+import (
+ "dubbo.apache.org/dubbo-go/v3/config"
+
+ hessian "github.com/apache/dubbo-go-hessian2"
+)
+
+func init() {
+ config.SetProviderService(new(UserProvider))
+ // ------for hessian2------
+ hessian.RegisterPOJO(&User{})
+
+ cache = &UserDB{
+ nameIndex: make(map[string]*User, 16),
+ codeIndex: make(map[int64]*User, 16),
+ lock: sync.Mutex{},
+ }
+
+ cache.Add(&User{ID: "0001", Code: 1, Name: "tc", Age: 18, Time: time.Now()})
+ cache.Add(&User{ID: "0002", Code: 2, Name: "ic", Age: 88, Time: time.Now()})
+}
+
+var cache *UserDB
+
+// UserDB cache user.
+type UserDB struct {
+ // key is name, value is user obj
+ nameIndex map[string]*User
+ // key is code, value is user obj
+ codeIndex map[int64]*User
+ lock sync.Mutex
+}
+
+// nolint
+func (db *UserDB) Add(u *User) bool {
+ if u.Name == "" || u.Code <= 0 {
+ return false
+ }
+
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ if !db.existName(u.Name) && !db.existCode(u.Code) {
+ return db.AddForName(u) && db.AddForCode(u)
+ }
+
+ return false
+}
+
+// nolint
+func (db *UserDB) AddForName(u *User) bool {
+ if len(u.Name) == 0 {
+ return false
+ }
+
+ if _, ok := db.nameIndex[u.Name]; ok {
+ return false
+ }
+
+ db.nameIndex[u.Name] = u
+ return true
+}
+
+// nolint
+func (db *UserDB) AddForCode(u *User) bool {
+ if u.Code <= 0 {
+ return false
+ }
+
+ if _, ok := db.codeIndex[u.Code]; ok {
+ return false
+ }
+
+ db.codeIndex[u.Code] = u
+ return true
+}
+
+// nolint
+func (db *UserDB) GetByName(n string) (*User, bool) {
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ r, ok := db.nameIndex[n]
+ return r, ok
+}
+
+// nolint
+func (db *UserDB) GetByCode(n int64) (*User, bool) {
+ db.lock.Lock()
+ defer db.lock.Unlock()
+
+ r, ok := db.codeIndex[n]
+ return r, ok
+}
+
+func (db *UserDB) existName(name string) bool {
+ if len(name) <= 0 {
+ return false
+ }
+
+ _, ok := db.nameIndex[name]
+ if ok {
+ return true
+ }
+
+ return false
+}
+
+func (db *UserDB) existCode(code int64) bool {
+ if code <= 0 {
+ return false
+ }
+
+ _, ok := db.codeIndex[code]
+ if ok {
+ return true
+ }
+
+ return false
+}
+
+// User user obj.
+type User struct {
+ ID string `json:"id,omitempty"`
+ Code int64 `json:"code,omitempty"`
+ Name string `json:"name,omitempty"`
+ Age int32 `json:"age,omitempty"`
+ Time time.Time `json:"time,omitempty"`
+}
+
+// UserProvider the dubbo provider.
+// like: version: 1.0.0 group: test
+type UserProvider struct{}
+
+// CreateUser new user, PX config POST.
+func (u *UserProvider) CreateUser(ctx context.Context, user *User) (*User, error) {
+ outLn("Req CreateUser data:%#v", user)
+ if user == nil {
+ return nil, errors.New("not found")
+ }
+ _, ok := cache.GetByName(user.Name)
+ if ok {
+ return nil, errors.New("data is exist")
+ }
+
+ b := cache.Add(user)
+ if b {
+ return user, nil
+ }
+
+ return nil, errors.New("add error")
+}
+
+// GetUserByName query by name, single param, PX config GET.
+func (u *UserProvider) GetUserByName(ctx context.Context, name string) (*User, error) {
+ outLn("Req GetUserByName name:%#v", name)
+ r, ok := cache.GetByName(name)
+ if ok {
+ outLn("Req GetUserByName result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserByCode query by code, single param, PX config GET.
+func (u *UserProvider) GetUserByCode(ctx context.Context, code int64) (*User, error) {
+ outLn("Req GetUserByCode name:%#v", code)
+ r, ok := cache.GetByCode(code)
+ if ok {
+ outLn("Req GetUserByCode result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserTimeout query by name, will timeout for pixiu.
+func (u *UserProvider) GetUserTimeout(ctx context.Context, name string) (*User, error) {
+ outLn("Req GetUserByName name:%#v", name)
+ // sleep 10s, pixiu config less than 10s.
+ time.Sleep(10 * time.Second)
+ r, ok := cache.GetByName(name)
+ if ok {
+ outLn("Req GetUserByName result:%#v", r)
+ return r, nil
+ }
+ return nil, nil
+}
+
+// GetUserByNameAndAge query by name and age, two params, PX config GET.
+func (u *UserProvider) GetUserByNameAndAge(ctx context.Context, name string, age int32) (*User, error) {
+ outLn("Req GetUserByNameAndAge name:%s, age:%d", name, age)
+ r, ok := cache.GetByName(name)
+ if ok && r.Age == age {
+ outLn("Req GetUserByNameAndAge result:%#v", r)
+ return r, nil
+ }
+ return r, nil
+}
+
+// UpdateUser update by user struct, my be another struct, PX config POST or PUT.
+func (u *UserProvider) UpdateUser(ctx context.Context, user *User) (bool, error) {
+ outLn("Req UpdateUser data:%#v", user)
+ r, ok := cache.GetByName(user.Name)
+ if ok {
+ if user.ID != "" {
+ r.ID = user.ID
+ }
+ if user.Age >= 0 {
+ r.Age = user.Age
+ }
+ return true, nil
+ }
+ return false, errors.New("not found")
+}
+
+// UpdateUserByName update by user struct, my be another struct, PX config POST or PUT.
+func (u *UserProvider) UpdateUserByName(ctx context.Context, name string, user *User) (bool, error) {
+ outLn("Req UpdateUserByName data:%#v", user)
+ r, ok := cache.GetByName(name)
+ if ok {
+ if user.ID != "" {
+ r.ID = user.ID
+ }
+ if user.Age >= 0 {
+ r.Age = user.Age
+ }
+ return true, nil
+ }
+ return false, errors.New("not found")
+}
+
+// nolint
+func (u *UserProvider) Reference() string {
+ return "UserProvider"
+}
+
+// nolint
+func (u User) JavaClassName() string {
+ return "com.dubbogo.pixiu.User"
+}
+
+// nolint
+func outLn(format string, args ...interface{}) {
+ fmt.Printf("\033[32;40m"+format+"\033[0m\n", args...)
Review Comment:
If u really wannt use the color prefix, pls define them as const in common/color.go. Follow the following example from my c lib lisk
```go
static const n1* color_string_[] = {
[ESCREEN_COLOR_MIN] = "",
[ENORMAL] = "\033[0m",
[EBLUE] = "\033[1;34;40m",
[ERED] = "\033[1;31;40m",
[EPRUPLE] = "\033[1;35;40m",
[EWHITE] = "\033[1;37;40m",
[EGREEN] = "\033[1;32;40m",
[EAQUA] = "\033[1;36;40m",
[EHIGH_LIGHT] = "\033[1;48;40m",
[EYELLOW] = "\033[1;33;40m",
[EUNDER_LINE] = "\033[1;4;40m",
[EUNDER_LINE_TWINKLE] = "\033[1;5;40m",
[EUNDER_LINE_TWINKLE_HIGH_LIGHT] = "\033[1;7;40m"
};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r846550699
##########
pkg/adapter/dubboregistry/registry/nacos/interface_listener.go:
##########
@@ -146,53 +151,74 @@ func fromServiceFullKey(fullKey string) *serviceInfo {
}
func (z *nacosIntfListener) updateServiceList(serviceList []string) error {
- // todo lock all svc listener
+ // add new service info and watch
+
+ newServiceMap := make(map[string]bool)
- allSvcListener := z.reg.GetAllSvcListener()
- subscribedServiceKeysMap := make(map[string]bool)
- for k := range allSvcListener {
- subscribedServiceKeysMap[k] = true
- }
- serviceNeedUpdate := make([]*serviceInfo, 0)
for _, v := range serviceList {
svcInfo := fromServiceFullKey(v)
if svcInfo == nil {
// invalid nacos dubbo service key
continue
}
key := svcInfo.String()
- if _, ok := allSvcListener[key]; !ok {
- serviceNeedUpdate = append(serviceNeedUpdate, svcInfo)
- } else {
- delete(subscribedServiceKeysMap, key)
+ newServiceMap[key] = true
+ if _, ok := z.serviceInfoMap[key]; !ok {
+
+ url, _ := dubboCommon.NewURL("mock://localhost:8848")
+ url.SetParam(constant.InterfaceKey, svcInfo.interfaceName)
+ url.SetParam(constant.GroupKey, svcInfo.group)
+ url.SetParam(constant.VersionKey, svcInfo.version)
+ l := newNacosSrvListener(url, z.client, z.adapterListener)
+ l.wg.Add(1)
+
+ svcInfo.listener = l
+ z.serviceInfoMap[key] = svcInfo
+
+ go func(v *serviceInfo) {
+ defer l.wg.Done()
Review Comment:
已经修改，在 service_listener.go 的 serviceListener.Close 函数中 wait。updateServiceList 函数中有涉及对比内存缓存的serviceList 和新获取 serviceList的逻辑，会进行 service_listener 的删除操作 195L，会调用到 Close 函数。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r846550208
##########
pkg/client/dubbo/dubbo.go:
##########
@@ -323,7 +323,7 @@ func (dc *Client) create(key string, irequest fc.IntegrationRequest) *generic.Ge
// sleep when first call to fetch enough service meta data from nacos
// todo: GenericLoad should guarantee it
if useNacosRegister {
Review Comment:
已经修改，按照 RegistryConfig.Protocol == nacos 进行判断
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] ztelur commented on a diff in pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389#discussion_r846550208
##########
pkg/client/dubbo/dubbo.go:
##########
@@ -323,7 +323,7 @@ func (dc *Client) create(key string, irequest fc.IntegrationRequest) *generic.Ge
// sleep when first call to fetch enough service meta data from nacos
// todo: GenericLoad should guarantee it
if useNacosRegister {
Review Comment:
按照 RegistryConfig.Protocol == nacos 进行判断
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
[GitHub] [dubbo-go-pixiu] AlexStocks merged pull request #389: Bugfix nacos registry bug
Posted by GitBox <gi...@apache.org>.
AlexStocks merged PR #389:
URL: https://github.com/apache/dubbo-go-pixiu/pull/389
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org