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/06/08 15:34:17 UTC

[GitHub] [dubbo-go-pixiu] MasterKenway opened a new pull request, #440: Fix/nacos cluster

MasterKenway opened a new pull request, #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440

   <!--  Thanks for sending a pull request! 
   -->
   
   **What this PR does**:
   
   **Which issue(s) this PR fixes**:
   <!--
   *Automatically closes linked issue when PR is merged.
   Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
   _If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
   -->
   Fixes #
   
   **Special notes for your reviewer**:
   
   **Does this PR introduce a user-facing change?**:
   <!--
   If no, just write "NONE" in the release-note block below.
   If yes, a release note is required:
   Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
   -->
   ```release-note
   
   ```


-- 
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 #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440#issuecomment-1153163192

   # [Codecov](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440?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 [#440](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b1c205a) into [develop](https://codecov.io/gh/apache/dubbo-go-pixiu/commit/0a54226fce5bf308218e0ebcd312174ffd892f46?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0a54226) will **decrease** coverage by `0.41%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           develop     #440      +/-   ##
   ===========================================
   - Coverage    37.62%   37.20%   -0.42%     
   ===========================================
     Files           55       55              
     Lines         3748     3790      +42     
   ===========================================
     Hits          1410     1410              
   - Misses        2180     2220      +40     
   - Partials       158      160       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440?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/router/route.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440/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-cGtnL3JvdXRlci9yb3V0ZS5nbw==) | `45.73% <0.00%> (-13.80%)` | :arrow_down: |
   | [pkg/server/cluster\_manager.go](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440/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.41% <0.00%> (-1.05%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440?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/440?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 [0a54226...b1c205a](https://codecov.io/gh/apache/dubbo-go-pixiu/pull/440?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 #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
ztelur commented on code in PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440#discussion_r894979014


##########
pkg/common/util/stringutil/stringutil.go:
##########
@@ -18,6 +18,8 @@
 package stringutil
 
 import (
+	perrors "github.com/pkg/errors"

Review Comment:
   use ` imports-formatter -path . -module github.com/apache/dubbo-go-pixiu -bl false` to auto group imports line



##########
pkg/router/route.go:
##########
@@ -58,12 +59,54 @@ func (rt *Route) ClearAPI() error {
 	return nil
 }
 
-func (r *Route) RemoveAPI(api router.API) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
+func (rt *Route) RemoveAPI(api router.API) {
 	lowerCasePath := strings.ToLower(api.URLPattern)
 	key := getTrieKey(api.Method.HTTPVerb, lowerCasePath, false)
-	_, _ = r.tree.Remove(key)
+
+	rt.lock.Lock()
+	defer rt.lock.Unlock()
+	// if api is exists
+	if exists, err := rt.tree.Contains(key); err != nil {
+		logger.Errorf("rt.tree.Get(key) err: %s", err.Error())
+		return
+	} else if exists {
+		// append new cluster to the node

Review Comment:
   这部分代码修改的背景或者目的是?



-- 
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] MasterKenway commented on a diff in pull request #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
MasterKenway commented on code in PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440#discussion_r894984121


##########
pkg/router/route.go:
##########
@@ -58,12 +59,54 @@ func (rt *Route) ClearAPI() error {
 	return nil
 }
 
-func (r *Route) RemoveAPI(api router.API) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
+func (rt *Route) RemoveAPI(api router.API) {
 	lowerCasePath := strings.ToLower(api.URLPattern)
 	key := getTrieKey(api.Method.HTTPVerb, lowerCasePath, false)
-	_, _ = r.tree.Remove(key)
+
+	rt.lock.Lock()
+	defer rt.lock.Unlock()
+	// if api is exists
+	if exists, err := rt.tree.Contains(key); err != nil {
+		logger.Errorf("rt.tree.Get(key) err: %s", err.Error())
+		return
+	} else if exists {
+		// append new cluster to the node

Review Comment:
   我们的负载均衡是根据 Trie Tree 叶子节点中的 mehtod.URL 属性来获取服务提供者,原本单纯的将叶子节点进行替换会导致之前订阅到的服务器提供者被覆盖,这里的删除则是避免其中一个服务器提供者下线后,不导致所有对端服务都从 Trie Tree中移除(这里的注释直接复制了上面的代码,我会在后面的 commit 修改)



-- 
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] MasterKenway commented on a diff in pull request #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
MasterKenway commented on code in PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440#discussion_r894984121


##########
pkg/router/route.go:
##########
@@ -58,12 +59,54 @@ func (rt *Route) ClearAPI() error {
 	return nil
 }
 
-func (r *Route) RemoveAPI(api router.API) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
+func (rt *Route) RemoveAPI(api router.API) {
 	lowerCasePath := strings.ToLower(api.URLPattern)
 	key := getTrieKey(api.Method.HTTPVerb, lowerCasePath, false)
-	_, _ = r.tree.Remove(key)
+
+	rt.lock.Lock()
+	defer rt.lock.Unlock()
+	// if api is exists
+	if exists, err := rt.tree.Contains(key); err != nil {
+		logger.Errorf("rt.tree.Get(key) err: %s", err.Error())
+		return
+	} else if exists {
+		// append new cluster to the node

Review Comment:
   我们的负载均衡是根据 Trie Tree 叶子节点中的 mehtod.URL 属性来获取服务提供者,原本单纯的将叶子节点进行替换会导致之前订阅到的服务器提供者被覆盖,这里的删除则是避免其中一个服务器提供者下线后,不导致所有对端服务都从 Trie Tree中移除



-- 
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 #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
AlexStocks merged PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440


-- 
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] MasterKenway commented on a diff in pull request #440: Fix/nacos cluster

Posted by GitBox <gi...@apache.org>.
MasterKenway commented on code in PR #440:
URL: https://github.com/apache/dubbo-go-pixiu/pull/440#discussion_r894984121


##########
pkg/router/route.go:
##########
@@ -58,12 +59,54 @@ func (rt *Route) ClearAPI() error {
 	return nil
 }
 
-func (r *Route) RemoveAPI(api router.API) {
-	r.lock.Lock()
-	defer r.lock.Unlock()
+func (rt *Route) RemoveAPI(api router.API) {
 	lowerCasePath := strings.ToLower(api.URLPattern)
 	key := getTrieKey(api.Method.HTTPVerb, lowerCasePath, false)
-	_, _ = r.tree.Remove(key)
+
+	rt.lock.Lock()
+	defer rt.lock.Unlock()
+	// if api is exists
+	if exists, err := rt.tree.Contains(key); err != nil {
+		logger.Errorf("rt.tree.Get(key) err: %s", err.Error())
+		return
+	} else if exists {
+		// append new cluster to the node

Review Comment:
   我们的负载均衡是根据 Trie Tree 叶子节点中的 mehtod.URL 属性来获取服务提供者,原本单纯的将叶子节点进行替换会导致之前订阅到的服务器提供者被覆盖



-- 
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