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 2020/07/15 13:14:53 UTC
[GitHub] [dubbo-go] DogBaoBao opened a new pull request #659: fix: Nearest first for multiple registry
DogBaoBao opened a new pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
This extension provides a strategy to decide how to distribute traffics among them:
1. registry marked as 'preferred=true' has the highest priority.
2. check the zone the current request belongs, pick the registry that has the same zone first.
3. Evenly balance traffic between all registries based on each registry's weight.
4. Pick anyone that's available.
**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 #597
**Special notes for your reviewer**:
@zouyx
**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.
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] DogBaoBao commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-664130756
> I think it should be a new feature. But the original extension `registry_aware_cluster_invoker` should be hold back?
This feature base on java implementation，no registryAware but zoneAware only.
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r462918881
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() && "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
Review comment:
ok, have some code format?
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r463345413
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,102 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
Review comment:
Thanks, but if by if make the code looks cumbersome. I think create a function is more elegant.
```go
key := constant.REGISTRY_KEY + "." + constant.PREFERRED_KEY
if invoker.IsAvailable() && matchParam("true", key, "false", invoker) {
return invoker.Invoke(ctx, invocation)
}
```
```go
func matchParam(target, key, def string, invoker protocol.Invoker) bool {
return target == invoker.GetUrl().GetParam(key, def)
}
```
----------------------------------------------------------------
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.
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] zouyx merged pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx merged pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r463345413
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,102 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
Review comment:
Thanks
----------------------------------------------------------------
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.
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] AlexStocks commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r463055945
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,102 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
+ }
+
+ // providers in the registry with the same zone
+ zone := invocation.AttachmentsByKey(constant.REGISTRY_ZONE, "")
+ if "" != zone {
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ zone == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.ZONE_KEY, "") {
+ return invoker.Invoke(ctx, invocation)
+ }
+ }
Review comment:
```Go
if invoker.IsAvailable() {
if zone == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.ZONE_KEY, "") {
return invoker.Invoke(ctx, invocation)
}
}
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r467371410
##########
File path: cluster/cluster_interceptor.go
##########
@@ -0,0 +1,37 @@
+/*
+ * 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 cluster
+
+import "context"
Review comment:
```suggestion
import (
"context"
)
```
##########
File path: cluster/cluster_impl/base_cluster_invoker.go
##########
@@ -17,6 +17,8 @@
package cluster_impl
+import "context"
Review comment:
```suggestion
import (
"context"
)
```
----------------------------------------------------------------
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.
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] AlexStocks commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r463055457
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,102 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
Review comment:
so ugly, maybe u can write ur codes as follows
```Go
if invoker.IsAvailable() {
if "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
return invoker.Invoke(ctx, invocation)
}
}
----------------------------------------------------------------
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.
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] AlexStocks commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r460363065
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,95 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "github.com/apache/dubbo-go/cluster"
Review comment:
pls split it.
##########
File path: cluster/loadbalance/util.go
##########
@@ -17,34 +17,43 @@
package loadbalance
-import (
- "time"
-)
-
import (
"github.com/apache/dubbo-go/common/constant"
"github.com/apache/dubbo-go/protocol"
+ "time"
Review comment:
pls move it to the first block as old.
##########
File path: config/reference_config.go
##########
@@ -20,6 +20,7 @@ package config
import (
"context"
"fmt"
+ "github.com/apache/dubbo-go/cluster/cluster_impl"
Review comment:
move it to another import block.
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,187 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
Review comment:
pls split it as other dubbo-go source code files.
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466798664
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
I think I need to add a field in RegistryConfig, like ZoneForce
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466822218
##########
File path: cluster/loadbalance/util.go
##########
@@ -28,23 +28,35 @@ import (
// GetWeight gets weight for load balance strategy
func GetWeight(invoker protocol.Invoker, invocation protocol.Invocation) int64 {
+ var weight int64
url := invoker.GetUrl()
- weight := url.GetMethodParamInt64(invocation.MethodName(), constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
+ // Multiple registry scenario, load balance among multiple registries.
+ isRegIvk := url.GetParamBool(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, false)
+ if isRegIvk {
+ weight = url.GetParamInt(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
+ } else {
+ weight = url.GetMethodParamInt64(invocation.MethodName(), constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
- if weight > 0 {
- //get service register time an do warm up time
- now := time.Now().Unix()
- timestamp := url.GetParamInt(constant.REMOTE_TIMESTAMP_KEY, now)
- if uptime := now - timestamp; uptime > 0 {
- warmup := url.GetParamInt(constant.WARMUP_KEY, constant.DEFAULT_WARMUP)
- if uptime < warmup {
- if ww := float64(uptime) / float64(warmup) / float64(weight); ww < 1 {
- weight = 1
- } else if int64(ww) <= weight {
- weight = int64(ww)
+ if weight > 0 {
+ //get service register time an do warm up time
+ now := time.Now().Unix()
+ timestamp := url.GetParamInt(constant.REMOTE_TIMESTAMP_KEY, now)
+ if uptime := now - timestamp; uptime > 0 {
+ warmup := url.GetParamInt(constant.WARMUP_KEY, constant.DEFAULT_WARMUP)
+ if uptime < warmup {
Review comment:
I'm not sure, the logic I changed has not affected warnup. Functionally speaking, I think this feature is still needed.
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466783293
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
I can‘t find where do you set this `REGISTRY_ZONE_FORCE` key?
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466798664
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
Because this tag is from consumer’s attachments. Need consumer specify by self. I only run this in testing TestZoneWareInvokerWithZoneForceFail.
----------------------------------------------------------------
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.
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] AlexStocks commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r462738891
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() && "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
+ }
+
+ // providers in the registry with the same zone
+ zone := invocation.AttachmentsByKey(constant.REGISTRY_ZONE, "")
+ if "" != zone {
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() && zone == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.ZONE_KEY, "") {
Review comment:
split it.
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,193 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "testing"
+)
+
+import (
+ "github.com/golang/mock/gomock"
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster/directory"
+ "github.com/apache/dubbo-go/common"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+ "github.com/apache/dubbo-go/protocol/invocation"
+ "github.com/apache/dubbo-go/protocol/mock"
+)
+
+func Test_ZoneWareInvokerWithPreferredSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ mockResult := &protocol.RPCResult{Attrs: map[string]string{constant.PREFERRED_KEY: "true"}, Rest: rest{tried: 0, success: true}}
Review comment:
long line . split it.
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,193 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "testing"
+)
+
+import (
+ "github.com/golang/mock/gomock"
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster/directory"
+ "github.com/apache/dubbo-go/common"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+ "github.com/apache/dubbo-go/protocol/invocation"
+ "github.com/apache/dubbo-go/protocol/mock"
+)
+
+func Test_ZoneWareInvokerWithPreferredSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ mockResult := &protocol.RPCResult{Attrs: map[string]string{constant.PREFERRED_KEY: "true"}, Rest: rest{tried: 0, success: true}}
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ if 0 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "true")
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return mockResult
+ })
+ } else {
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{}
+ })
+ }
+
+ invokers = append(invokers, invoker)
+ }
+
+ zoneAwareCluster := NewZoneAwareCluster()
+ staticDir := directory.NewStaticDirectory(invokers)
+ clusterInvoker := zoneAwareCluster.Join(staticDir)
+
+ result := clusterInvoker.Invoke(context.Background(), &invocation.RPCInvocation{})
+
+ assert.Equal(t, mockResult, result)
+}
+
+func Test_ZoneWareInvokerWithWeightSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ w1 := "50"
+ w2 := "200"
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, "true")
+ if 1 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, w1)
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.WEIGHT_KEY: w1}, Rest: rest{tried: 0, success: true}}
+ }).MaxTimes(100)
+ } else {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, w2)
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.WEIGHT_KEY: w2}, Rest: rest{tried: 0, success: true}}
+ }).MaxTimes(100)
+ }
+ invokers = append(invokers, invoker)
+ }
+
+ zoneAwareCluster := NewZoneAwareCluster()
+ staticDir := directory.NewStaticDirectory(invokers)
+ clusterInvoker := zoneAwareCluster.Join(staticDir)
+
+ var w2Count, w1Count int
+ loop := 50
+ for i := 0; i < loop; i++ {
+ result := clusterInvoker.Invoke(context.Background(), &invocation.RPCInvocation{})
+ if w2 == result.Attachment(constant.WEIGHT_KEY, "0") {
+ w2Count++
+ }
+ if w1 == result.Attachment(constant.WEIGHT_KEY, "0") {
+ w1Count++
+ }
+ assert.NoError(t, result.Error())
+ }
+ t.Logf("loop count : %d, w1 height : %s | count : %d, w2 height : %s | count : %d", loop,
+ w1, w1Count, w2, w2Count)
+}
+
+func Test_ZoneWareInvokerWithZoneSuccess(t *testing.T) {
+ var zoneArray = []string{"hangzhou", "shanghai"}
+
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ zoneValue := zoneArray[i]
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.ZONE_KEY, zoneValue)
+
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.ZONE_KEY: zoneValue}, Rest: rest{tried: 0, success: true}}
Review comment:
split
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() && "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
Review comment:
so long, can u split it into multiple lines?
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,193 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "testing"
+)
+
+import (
+ "github.com/golang/mock/gomock"
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster/directory"
+ "github.com/apache/dubbo-go/common"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+ "github.com/apache/dubbo-go/protocol/invocation"
+ "github.com/apache/dubbo-go/protocol/mock"
+)
+
+func Test_ZoneWareInvokerWithPreferredSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ mockResult := &protocol.RPCResult{Attrs: map[string]string{constant.PREFERRED_KEY: "true"}, Rest: rest{tried: 0, success: true}}
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ if 0 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "true")
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return mockResult
+ })
+ } else {
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{}
+ })
+ }
+
+ invokers = append(invokers, invoker)
+ }
+
+ zoneAwareCluster := NewZoneAwareCluster()
+ staticDir := directory.NewStaticDirectory(invokers)
+ clusterInvoker := zoneAwareCluster.Join(staticDir)
+
+ result := clusterInvoker.Invoke(context.Background(), &invocation.RPCInvocation{})
+
+ assert.Equal(t, mockResult, result)
+}
+
+func Test_ZoneWareInvokerWithWeightSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ w1 := "50"
+ w2 := "200"
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, "true")
+ if 1 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, w1)
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.WEIGHT_KEY: w1}, Rest: rest{tried: 0, success: true}}
Review comment:
split
##########
File path: config/registry_config.go
##########
@@ -40,11 +40,17 @@ type RegistryConfig struct {
TimeoutStr string `yaml:"timeout" default:"5s" json:"timeout,omitempty" property:"timeout"` // unit: second
Group string `yaml:"group" json:"group,omitempty" property:"group"`
// for registry
- Address string `yaml:"address" json:"address,omitempty" property:"address"`
- Username string `yaml:"username" json:"username,omitempty" property:"username"`
- Password string `yaml:"password" json:"password,omitempty" property:"password"`
- Simplified bool `yaml:"simplified" json:"simplified,omitempty" property:"simplified"`
- Params map[string]string `yaml:"params" json:"params,omitempty" property:"params"`
+ Address string `yaml:"address" json:"address,omitempty" property:"address"`
+ Username string `yaml:"username" json:"username,omitempty" property:"username"`
+ Password string `yaml:"password" json:"password,omitempty" property:"password"`
+ Simplified bool `yaml:"simplified" json:"simplified,omitempty" property:"simplified"`
+ // Always use this registry first if set to true, useful when subscribe to multiple registries
+ Preferred bool `yaml:"preferred" json:"params,omitempty" property:"preferred"`
+ // The region where the registry belongs, usually used to isolate traffics
+ Zone string `yaml:"zone" json:"params,omitempty" property:"zone"`
+ // Affects traffic distribution among registries, useful when subscribe to multiple registries Take effect only when no preferred registry is specified.
Review comment:
split it.
##########
File path: config/reference_config.go
##########
@@ -148,10 +149,21 @@ func (c *ReferenceConfig) Refer(_ interface{}) {
// TODO(decouple from directory, config should not depend on directory module)
if regUrl != nil {
- cluster := extension.GetCluster("registryAware")
+ // for multi-subscription scenario, use 'zone-aware' policy by default
+ cluster := extension.GetCluster(cluster_impl.GetZoneAwareName())
+ // The invoker wrap sequence would be: ZoneAwareClusterInvoker(StaticDirectory) -> FailoverClusterInvoker(RegistryDirectory, routing happens here) -> Invoker
Review comment:
split it.
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,193 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "testing"
+)
+
+import (
+ "github.com/golang/mock/gomock"
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster/directory"
+ "github.com/apache/dubbo-go/common"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+ "github.com/apache/dubbo-go/protocol/invocation"
+ "github.com/apache/dubbo-go/protocol/mock"
+)
+
+func Test_ZoneWareInvokerWithPreferredSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ mockResult := &protocol.RPCResult{Attrs: map[string]string{constant.PREFERRED_KEY: "true"}, Rest: rest{tried: 0, success: true}}
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ if 0 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "true")
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return mockResult
+ })
+ } else {
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{}
+ })
+ }
+
+ invokers = append(invokers, invoker)
+ }
+
+ zoneAwareCluster := NewZoneAwareCluster()
+ staticDir := directory.NewStaticDirectory(invokers)
+ clusterInvoker := zoneAwareCluster.Join(staticDir)
+
+ result := clusterInvoker.Invoke(context.Background(), &invocation.RPCInvocation{})
+
+ assert.Equal(t, mockResult, result)
+}
+
+func Test_ZoneWareInvokerWithWeightSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ w1 := "50"
+ w2 := "200"
+
+ var invokers []protocol.Invoker
+ for i := 0; i < 2; i++ {
+ url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i))
+ invoker := mock.NewMockInvoker(ctrl)
+ invoker.EXPECT().IsAvailable().Return(true).AnyTimes()
+ invoker.EXPECT().GetUrl().Return(url).AnyTimes()
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, "true")
+ if 1 == i {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, w1)
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.WEIGHT_KEY: w1}, Rest: rest{tried: 0, success: true}}
+ }).MaxTimes(100)
+ } else {
+ url.SetParam(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, w2)
+ invoker.EXPECT().Invoke(gomock.Any()).DoAndReturn(
+ func(invocation protocol.Invocation) protocol.Result {
+ return &protocol.RPCResult{Attrs: map[string]string{constant.WEIGHT_KEY: w2}, Rest: rest{tried: 0, success: true}}
Review comment:
split
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466853060
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
I think may be add ClusterInterceptor is needed.
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r467085660
##########
File path: cluster/cluster_impl/base_cluster_invoker.go
##########
@@ -18,6 +18,7 @@
package cluster_impl
import (
+ "context"
Review comment:
split pkg
##########
File path: cluster/cluster_interceptor.go
##########
@@ -0,0 +1,36 @@
+/*
+ * 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 cluster
+
+import (
+ "context"
+ "github.com/apache/dubbo-go/protocol"
Review comment:
split pkg
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466819648
##########
File path: config/reference_config.go
##########
@@ -148,10 +149,23 @@ func (c *ReferenceConfig) Refer(_ interface{}) {
// TODO(decouple from directory, config should not depend on directory module)
if regUrl != nil {
- cluster := extension.GetCluster("registryAware")
+ // for multi-subscription scenario, use 'zone-aware' policy by default
+ cluster := extension.GetCluster(cluster_impl.GetZoneAwareName())
+ // The invoker wrap sequence would be:
+ // ZoneAwareClusterInvoker(StaticDirectory) ->
+ // FailoverClusterInvoker(RegistryDirectory, routing happens here) -> Invoker
c.invoker = cluster.Join(directory.NewStaticDirectory(invokers))
} else {
- cluster := extension.GetCluster(c.Cluster)
+ // not a registry url, must be direct invoke.
+ clu := cluster_impl.GetFailoverName()
+ if len(invokers) > 0 {
+ u := invokers[0].GetUrl()
+ if nil != &u {
+ clu = u.GetParam(constant.CLUSTER_KEY, cluster_impl.GetZoneAwareName())
+ }
+ }
+
+ cluster := extension.GetCluster(clu)
c.invoker = cluster.Join(directory.NewStaticDirectory(invokers))
Review comment:
I think do following changes:
```go
var hitClu string
if regUrl != nil {
// for multi-subscription scenario, use 'zone-aware' policy by default
hitClu = constant.ZONEAWARE_CLUSTER_NAME
} else {
// not a registry url, must be direct invoke.
hitClu = constant.FAILOVER_CLUSTER_NAME
if len(invokers) > 0 {
u := invokers[0].GetUrl()
if nil != &u {
hitClu = u.GetParam(constant.CLUSTER_KEY, constant.ZONEAWARE_CLUSTER_NAME)
}
}
}
cluster := extension.GetCluster(hitClu)
// If 'zone-aware' policy select, the invoker wrap sequence would be:
// ZoneAwareClusterInvoker(StaticDirectory) ->
// FailoverClusterInvoker(RegistryDirectory, routing happens here) -> Invoker
c.invoker = cluster.Join(directory.NewStaticDirectory(invokers))
```
----------------------------------------------------------------
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.
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] DogBaoBao commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-664744908
> Have u finished you pr? Plc merge the latest develop branch to fix the conflict problembs.
Finish and merge develop branch.
----------------------------------------------------------------
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.
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] codecov-commenter commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-671010688
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=h1) Report
> Merging [#659](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=desc) into [develop](https://codecov.io/gh/apache/dubbo-go/commit/5e377f4db998883857a07afa59546c95d387babe&el=desc) will **increase** coverage by `0.34%`.
> The diff coverage is `74.46%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #659 +/- ##
===========================================
+ Coverage 63.58% 63.93% +0.34%
===========================================
Files 239 238 -1
Lines 12650 12487 -163
===========================================
- Hits 8044 7983 -61
+ Misses 3821 3740 -81
+ Partials 785 764 -21
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/loadbalance/util.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9sb2FkYmFsYW5jZS91dGlsLmdv) | `66.66% <61.11%> (-19.05%)` | :arrow_down: |
| [cluster/cluster\_impl/zone\_aware\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `68.00% <68.00%> (ø)` | |
| [cluster/cluster\_impl/base\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYmFzZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `72.15% <85.71%> (+1.31%)` | :arrow_up: |
| [cluster/cluster\_impl/failover\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvZmFpbG92ZXJfY2x1c3Rlci5nbw==) | `100.00% <100.00%> (ø)` | |
| [cluster/cluster\_impl/zone\_aware\_cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvem9uZV9hd2FyZV9jbHVzdGVyLmdv) | `100.00% <100.00%> (ø)` | |
| [config/reference\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y29uZmlnL3JlZmVyZW5jZV9jb25maWcuZ28=) | `79.82% <100.00%> (+0.92%)` | :arrow_up: |
| [config/registry\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y29uZmlnL3JlZ2lzdHJ5X2NvbmZpZy5nbw==) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `71.42% <0.00%> (-12.79%)` | :arrow_down: |
| [config/config\_center\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y29uZmlnL2NvbmZpZ19jZW50ZXJfY29uZmlnLmdv) | `64.28% <0.00%> (-9.05%)` | :arrow_down: |
| [config/consumer\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree#diff-Y29uZmlnL2NvbnN1bWVyX2NvbmZpZy5nbw==) | `56.25% <0.00%> (ø)` | |
| ... and [11 more](https://codecov.io/gh/apache/dubbo-go/pull/659/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=footer). Last update [5e377f4...4dfc8aa](https://codecov.io/gh/apache/dubbo-go/pull/659?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] DogBaoBao commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-671041553
sample pr: https://github.com/dubbogo/dubbo-samples/pull/6
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r463345413
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker.go
##########
@@ -0,0 +1,102 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+)
+
+// When there're more than one registry for subscription.
+//
+// This extension provides a strategy to decide how to distribute traffics among them:
+// 1. registry marked as 'preferred=true' has the highest priority.
+// 2. check the zone the current request belongs, pick the registry that has the same zone first.
+// 3. Evenly balance traffic between all registries based on each registry's weight.
+// 4. Pick anyone that's available.
+type zoneAwareClusterInvoker struct {
+ baseClusterInvoker
+}
+
+func newZoneAwareClusterInvoker(directory cluster.Directory) protocol.Invoker {
+ return &zoneAwareClusterInvoker{
+ baseClusterInvoker: newBaseClusterInvoker(directory),
+ }
+}
+
+// nolint
+func (invoker *zoneAwareClusterInvoker) Invoke(ctx context.Context, invocation protocol.Invocation) protocol.Result {
+ invokers := invoker.directory.List(invocation)
+
+ err := invoker.checkInvokers(invokers, invocation)
+ if err != nil {
+ return &protocol.RPCResult{Err: err}
+ }
+
+ // First, pick the invoker (XXXClusterInvoker) that comes from the local registry, distinguish by a 'preferred' key.
+ for _, invoker := range invokers {
+ if invoker.IsAvailable() &&
+ "true" == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.PREFERRED_KEY, "false") {
+ return invoker.Invoke(ctx, invocation)
+ }
Review comment:
Thanks, but if by if make the code looks cumbersome. I think create a function is more elegant.
```go
if invoker.IsAvailable() && isTargetZone(zone, invoker) {
return invoker.Invoke(ctx, invocation)
}
```
```go
func isTargetZone(zone string, invoker protocol.Invoker) bool {
return zone == invoker.GetUrl().GetParam(constant.REGISTRY_KEY+"."+constant.ZONE_KEY, "")
}
```
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r462932453
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
Review comment:
It's missing..
----------------------------------------------------------------
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.
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] hxmhlt commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
hxmhlt commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r465783534
##########
File path: cluster/loadbalance/util.go
##########
@@ -28,23 +28,35 @@ import (
// GetWeight gets weight for load balance strategy
func GetWeight(invoker protocol.Invoker, invocation protocol.Invocation) int64 {
+ var weight int64
url := invoker.GetUrl()
- weight := url.GetMethodParamInt64(invocation.MethodName(), constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
+ // Multiple registry scenario, load balance among multiple registries.
+ isRegIvk := url.GetParamBool(constant.REGISTRY_KEY+"."+constant.REGISTRY_LABEL_KEY, false)
+ if isRegIvk {
+ weight = url.GetParamInt(constant.REGISTRY_KEY+"."+constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
+ } else {
+ weight = url.GetMethodParamInt64(invocation.MethodName(), constant.WEIGHT_KEY, constant.DEFAULT_WEIGHT)
- if weight > 0 {
- //get service register time an do warm up time
- now := time.Now().Unix()
- timestamp := url.GetParamInt(constant.REMOTE_TIMESTAMP_KEY, now)
- if uptime := now - timestamp; uptime > 0 {
- warmup := url.GetParamInt(constant.WARMUP_KEY, constant.DEFAULT_WARMUP)
- if uptime < warmup {
- if ww := float64(uptime) / float64(warmup) / float64(weight); ww < 1 {
- weight = 1
- } else if int64(ww) <= weight {
- weight = int64(ww)
+ if weight > 0 {
+ //get service register time an do warm up time
+ now := time.Now().Unix()
+ timestamp := url.GetParamInt(constant.REMOTE_TIMESTAMP_KEY, now)
+ if uptime := now - timestamp; uptime > 0 {
+ warmup := url.GetParamInt(constant.WARMUP_KEY, constant.DEFAULT_WARMUP)
+ if uptime < warmup {
Review comment:
Now I am not think warmup is used for golang. May we can remove the warmup logic. Isn't it?
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
I do not think '_' is golang native.
##########
File path: cluster/cluster_impl/failover_cluster.go
##########
@@ -44,3 +44,7 @@ func NewFailoverCluster() cluster.Cluster {
func (cluster *failoverCluster) Join(directory cluster.Directory) protocol.Invoker {
return newFailoverClusterInvoker(directory)
}
+
+func GetFailoverName() string {
Review comment:
Pls move the const into directory "constant"
##########
File path: config/reference_config.go
##########
@@ -26,6 +26,7 @@ import (
)
import (
+ "github.com/apache/dubbo-go/cluster/cluster_impl"
Review comment:
split
----------------------------------------------------------------
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.
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] hxmhlt commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
hxmhlt commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-659799149
I think it should be a new feature. But the original extension `registry_aware_cluster_invoker` should be hold back?
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466798664
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
I think I need to add a field in RegistryConfig, like ZoneForce
----------------------------------------------------------------
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.
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] AlexStocks commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-663806857
Have u finished you pr? Plc merge the latest develop branch to fix the conflict problembs.
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r462699190
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
Review comment:
No change?
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
+func NewZoneAwareCluster() cluster.Cluster {
+ return &zoneAwareCluster{}
}
-// NewRegistryAwareCluster returns a registry aware cluster instance
-func NewRegistryAwareCluster() cluster.Cluster {
- return ®istryAwareCluster{}
+func (cluster *zoneAwareCluster) Join(directory cluster.Directory) protocol.Invoker {
+ return newZoneAwareClusterInvoker(directory)
}
-// nolint
-func (cluster *registryAwareCluster) Join(directory cluster.Directory) protocol.Invoker {
- return newRegistryAwareClusterInvoker(directory)
+func GetZoneAwareName() string {
Review comment:
I think `// nolint` enough.
Comment format is `// methodName **************` like `// GetZoneAwareName gets cluster name`
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466821348
##########
File path: cluster/cluster_impl/failover_cluster.go
##########
@@ -44,3 +44,7 @@ func NewFailoverCluster() cluster.Cluster {
func (cluster *failoverCluster) Join(directory cluster.Directory) protocol.Invoker {
return newFailoverClusterInvoker(directory)
}
+
+func GetFailoverName() string {
Review comment:
Other cluster_impl is def private name because they not used out package, failover and zoneAware is the special, used in reference_config for select or default set, I want to put the two constant to new file common/constant/cluster.go.
```go
const (
FAILOVER_CLUSTER_NAME = "failover"
ZONEAWARE_CLUSTER_NAME = "zoneAware"
)
```
How about this?
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r465823050
##########
File path: config/reference_config.go
##########
@@ -148,10 +149,23 @@ func (c *ReferenceConfig) Refer(_ interface{}) {
// TODO(decouple from directory, config should not depend on directory module)
if regUrl != nil {
- cluster := extension.GetCluster("registryAware")
+ // for multi-subscription scenario, use 'zone-aware' policy by default
+ cluster := extension.GetCluster(cluster_impl.GetZoneAwareName())
+ // The invoker wrap sequence would be:
+ // ZoneAwareClusterInvoker(StaticDirectory) ->
+ // FailoverClusterInvoker(RegistryDirectory, routing happens here) -> Invoker
c.invoker = cluster.Join(directory.NewStaticDirectory(invokers))
} else {
- cluster := extension.GetCluster(c.Cluster)
+ // not a registry url, must be direct invoke.
+ clu := cluster_impl.GetFailoverName()
+ if len(invokers) > 0 {
+ u := invokers[0].GetUrl()
+ if nil != &u {
+ clu = u.GetParam(constant.CLUSTER_KEY, cluster_impl.GetZoneAwareName())
+ }
+ }
+
+ cluster := extension.GetCluster(clu)
c.invoker = cluster.Join(directory.NewStaticDirectory(invokers))
Review comment:
i think this two logic can move out of `if-else`
----------------------------------------------------------------
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.
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] DogBaoBao commented on pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#issuecomment-659123225
> https://github.com/apache/dubbo-go/blob/master/contributing.md
OK，I will do better next.
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r466798664
##########
File path: common/constant/key.go
##########
@@ -97,6 +97,11 @@ const (
ROLE_KEY = "registry.role"
REGISTRY_DEFAULT_KEY = "registry.default"
REGISTRY_TIMEOUT_KEY = "registry.timeout"
+ REGISTRY_LABEL_KEY = "label"
+ PREFERRED_KEY = "preferred"
+ ZONE_KEY = "zone"
+ REGISTRY_ZONE = "registry_zone"
+ REGISTRY_ZONE_FORCE = "registry_zone_force"
Review comment:
Beacuse this tag is from consumer’s attachments. Need consumer specify by self. I only run this in testing TestZoneWareInvokerWithZoneForceFail.
----------------------------------------------------------------
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.
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] DogBaoBao commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
DogBaoBao commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r462922017
##########
File path: cluster/cluster_impl/zone_aware_cluster_invoker_test.go
##########
@@ -0,0 +1,193 @@
+/*
+ * 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 cluster_impl
+
+import (
+ "context"
+ "fmt"
+ "testing"
+)
+
+import (
+ "github.com/golang/mock/gomock"
+ "github.com/stretchr/testify/assert"
+)
+
+import (
+ "github.com/apache/dubbo-go/cluster/directory"
+ "github.com/apache/dubbo-go/common"
+ "github.com/apache/dubbo-go/common/constant"
+ "github.com/apache/dubbo-go/protocol"
+ "github.com/apache/dubbo-go/protocol/invocation"
+ "github.com/apache/dubbo-go/protocol/mock"
+)
+
+func Test_ZoneWareInvokerWithPreferredSuccess(t *testing.T) {
+ ctrl := gomock.NewController(t)
+ // In Go versions 1.14+, if you pass a *testing.T into gomock.NewController(t) you no longer need to call ctrl.Finish().
+ //defer ctrl.Finish()
+
+ mockResult := &protocol.RPCResult{Attrs: map[string]string{constant.PREFERRED_KEY: "true"}, Rest: rest{tried: 0, success: true}}
Review comment:
Yes, I will change
----------------------------------------------------------------
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.
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] zouyx commented on a change in pull request #659: Ftr: Nearest first for multiple registry
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #659:
URL: https://github.com/apache/dubbo-go/pull/659#discussion_r461304239
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
+func NewZoneAwareCluster() cluster.Cluster {
+ return &zoneAwareCluster{}
}
-// NewRegistryAwareCluster returns a registry aware cluster instance
-func NewRegistryAwareCluster() cluster.Cluster {
- return ®istryAwareCluster{}
+func (cluster *zoneAwareCluster) Join(directory cluster.Directory) protocol.Invoker {
Review comment:
add comment for all public methods
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
+func NewZoneAwareCluster() cluster.Cluster {
+ return &zoneAwareCluster{}
}
-// NewRegistryAwareCluster returns a registry aware cluster instance
-func NewRegistryAwareCluster() cluster.Cluster {
- return ®istryAwareCluster{}
+func (cluster *zoneAwareCluster) Join(directory cluster.Directory) protocol.Invoker {
+ return newZoneAwareClusterInvoker(directory)
}
-// nolint
-func (cluster *registryAwareCluster) Join(directory cluster.Directory) protocol.Invoker {
- return newRegistryAwareClusterInvoker(directory)
+func GetZoneAwareName() string {
Review comment:
// nolint
##########
File path: cluster/cluster_impl/zone_aware_cluster.go
##########
@@ -23,18 +23,23 @@ import (
"github.com/apache/dubbo-go/protocol"
)
-type registryAwareCluster struct{}
+type zoneAwareCluster struct{}
+
+const zoneAware = "zoneAware"
func init() {
- extension.SetCluster("registryAware", NewRegistryAwareCluster)
+ extension.SetCluster(zoneAware, NewZoneAwareCluster)
+}
+
+// NewZoneAwareCluster ...
Review comment:
```suggestion
// nolint
```
----------------------------------------------------------------
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.
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