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 2021/04/17 19:38:39 UTC

[GitHub] [dubbo-go] Patrick0308 opened a new pull request #1161: service discovery optimize

Patrick0308 opened a new pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161


   **What this PR does**:
   service discovery optimize
   
   


-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615339126



##########
File path: registry/event_listener.go
##########
@@ -26,31 +26,38 @@ import (
 )
 
 // The Service Discovery Changed  Event Listener
-type ServiceInstancesChangedListener struct {
+type ServiceInstancesChangedListenerBase struct {
 	ServiceName   string
 	ChangedNotify observer.ChangedNotify
 }
 
 // OnEvent on ServiceInstancesChangedEvent the service instances change event
-func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+func (lstn *ServiceInstancesChangedListenerBase) OnEvent(e observer.Event) error {
 	lstn.ChangedNotify.Notify(e)
 	return nil
 }
 
 // Accept return true if the name is the same
-func (lstn *ServiceInstancesChangedListener) Accept(e observer.Event) bool {
+func (lstn *ServiceInstancesChangedListenerBase) Accept(e observer.Event) bool {
 	if ce, ok := e.(*ServiceInstancesChangedEvent); ok {
 		return ce.ServiceName == lstn.ServiceName
 	}
 	return false
 }
 
 // GetPriority returns -1, it will be the first invoked listener
-func (lstn *ServiceInstancesChangedListener) GetPriority() int {
+func (lstn *ServiceInstancesChangedListenerBase) GetPriority() int {
 	return -1
 }
 
 // GetEventType returns ServiceInstancesChangedEvent
-func (lstn *ServiceInstancesChangedListener) GetEventType() reflect.Type {
+func (lstn *ServiceInstancesChangedListenerBase) GetEventType() reflect.Type {
 	return reflect.TypeOf(&ServiceInstancesChangedEvent{})
 }
+
+type ServiceInstanceChangeListener interface {

Review comment:
       lizhixin




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306749



##########
File path: metadata/service/inmemory/metadata_service_proxy_factory.go
##########
@@ -31,10 +32,30 @@ import (
 )
 
 func init() {
-	factory := service.NewBaseMetadataServiceProxyFactory(createProxy)
-	extension.SetMetadataServiceProxyFactory(local, func() service.MetadataServiceProxyFactory {
-		return factory
+	extension.SetLocalMetadataService(constant.DEFAULT_KEY, GetInMemoryMetadataService)
+	once = &sync.Once{}
+}
+
+var factory service.MetadataServiceProxyFactory

Review comment:
       ```go
   var (
       once *sync.Once
       factory service.MetadataServiceProxyFactory
   )
   ```




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305047



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),

Review comment:
       as I know the default value of a  uber atomic bool is false https://github.com/uber-go/atomic/blob/9c79392530c3be3273bd71db21239f2a5146c10c/bool.go#L41. so u need not to set its value again. pls delete this line.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615337296



##########
File path: common/extension/metadata_service.go
##########
@@ -26,42 +26,27 @@ import (
 )
 
 import (
-	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/common/constant"
 	"github.com/apache/dubbo-go/metadata/service"
 )
 
 var (
-	// there will be two types: local or remote
 	metadataServiceInsMap = make(map[string]func() (service.MetadataService, error), 2)

Review comment:
       localMetadataServiceInsMap




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338843



##########
File path: registry/event/base_configuration_listener.go
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 event
+
+import (
+	perrors "github.com/pkg/errors"
+)
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/config"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/config_center"
+	"github.com/apache/dubbo-go/remoting"
+)
+
+// nolint
+type BaseConfigurationListener struct {
+	configurators           []config_center.Configurator
+	dynamicConfiguration    config_center.DynamicConfiguration
+	defaultConfiguratorFunc func(url *common.URL) config_center.Configurator
+}
+
+// Configurators gets Configurator from config center
+func (bcl *BaseConfigurationListener) Configurators() []config_center.Configurator {
+	return bcl.configurators
+}
+
+// InitWith will init BaseConfigurationListener by @key+@Listener+@f
+func (bcl *BaseConfigurationListener) InitWith(key string, listener config_center.ConfigurationListener,
+	f func(url *common.URL) config_center.Configurator) {
+
+	bcl.dynamicConfiguration = config.GetEnvInstance().GetDynamicConfiguration()
+	if bcl.dynamicConfiguration == nil {
+		// set configurators to empty
+		bcl.configurators = []config_center.Configurator{}
+		return
+	}
+	bcl.defaultConfiguratorFunc = f
+	bcl.dynamicConfiguration.AddListener(key, listener)
+	if rawConfig, err := bcl.dynamicConfiguration.GetInternalProperty(key,
+		config_center.WithGroup(constant.DUBBO)); err != nil {
+		// set configurators to empty
+		bcl.configurators = []config_center.Configurator{}
+		return
+	} else if len(rawConfig) > 0 {
+		if err := bcl.genConfiguratorFromRawRule(rawConfig); err != nil {
+			logger.Error("bcl.genConfiguratorFromRawRule(rawConfig:%v) = error:%v", rawConfig, err)
+		}
+	}
+}
+
+// Process the notification event once there's any change happens on the config.
+func (bcl *BaseConfigurationListener) Process(event *config_center.ConfigChangeEvent) {
+	logger.Infof("Notification of overriding rule, change type is: %v , raw config content is:%v", event.ConfigType, event.Value)

Review comment:
       delete it.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615308141



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl

Review comment:
       !!! pls change the package name to remote_service or remote_impl




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615723403



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,251 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+import (
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY,
+	constant.TIMESTAMP_KEY,
+	constant.SERIALIZATION_KEY,
+	constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY,
+	constant.PATH_KEY,
+	constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY,
+	constant.VERSION_KEY,
+	constant.WARMUP_KEY,
+	constant.WEIGHT_KEY,
+	constant.RELEASE_KEY)
+
+// MetadataInfo the metadata information of instance
+type MetadataInfo struct {
+	App      string                  `json:"app,omitempty"`
+	Revision string                  `json:"revision,omitempty"`
+	Services map[string]*ServiceInfo `json:"services,omitempty"`
+
+	Reported *atomic.Bool `json:"-"`
+}
+
+// nolint
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+// nolint
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		Reported: atomic.NewBool(false),
+	}
+}
+
+// nolint
+func (mi *MetadataInfo) JavaClassName() string {
+	return "org.apache.dubbo.metadata.MetadataInfo"
+}
+
+// CalAndGetRevision is different from Dubbo because golang doesn't support overload
+// so that we should use interface + method name as identifier and ignore the method params
+// in my opinion, it's enough because Dubbo actually ignore the Url params.
+// please refer org.apache.dubbo.common.URL#toParameterString(java.lang.String...)
+func (mi *MetadataInfo) CalAndGetRevision() string {
+	if mi.Revision != "" && mi.Reported.Load() {
+		return mi.Revision
+	}
+	if len(mi.Services) == 0 {
+		return "0"
+	}
+	candidates := make([]string, 8)
+
+	for _, s := range mi.Services {
+		sk := s.ServiceKey
+		ms := s.Url.Methods
+		if len(ms) == 0 {
+			candidates = append(candidates, sk)
+		} else {
+			for _, m := range ms {
+				// methods are part of candidates
+				candidates = append(candidates, sk+constant.KEY_SEPARATOR+m)
+			}
+		}
+
+		// append Url params if we need it

Review comment:
       Url => URL




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305928



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl
+
+import (

Review comment:
       move it to the 3rd import block.




-- 
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] Patrick0308 commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615382812



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+			if "0" == revision {
+				logger.Infof("Find instance without valid service metadata: %s", instance.GetHost())
+				continue
+			}
+			subInstances := revisionToInstances[revision]
+			if subInstances == nil {
+				subInstances = make([]registry.ServiceInstance, 8)
+			}
+			revisionToInstances[revision] = append(subInstances, instance)
+			metadataInfo := lstn.revisionToMetadata[revision]

Review comment:
       I think don't need judge ok




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615308141



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl

Review comment:
       !!! pls change the package name to remote_service or remote_imp




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306011



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl
+
+import (
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/metadata/remote"
+	"github.com/apache/dubbo-go/registry"
+	"sync"
+)
+
+import (
+	"go.uber.org/atomic"
+)
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/definition"
+	"github.com/apache/dubbo-go/metadata/identifier"
+	"github.com/apache/dubbo-go/metadata/report/delegate"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+)
+
+// version will be used by Version func
+const (
+	version   = "1.0.0"

Review comment:
       may be u can use the constant Version in common/constant.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307651



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+			if "0" == revision {
+				logger.Infof("Find instance without valid service metadata: %s", instance.GetHost())
+				continue
+			}
+			subInstances := revisionToInstances[revision]
+			if subInstances == nil {
+				subInstances = make([]registry.ServiceInstance, 8)
+			}
+			revisionToInstances[revision] = append(subInstances, instance)
+			metadataInfo := lstn.revisionToMetadata[revision]
+			if metadataInfo == nil {
+				metadataInfo, err = lstn.getMetadataInfo(instance, metadataInfo, revision)
+				if err != nil {
+					return err
+				}
+			}
+			instance.SetServiceMetadata(metadataInfo)
+			for _, service := range metadataInfo.Services {
+				if localServiceToRevisions[service] == nil {
+					localServiceToRevisions[service] = gxset.NewSet()
+				}
+				localServiceToRevisions[service].Add(revision)
+			}
+
+			newRevisionToMetadata[revision] = metadataInfo
+		}
+		lstn.revisionToMetadata = newRevisionToMetadata
+
+		for serviceInstance, revisions := range localServiceToRevisions {
+			revisionsToUrls := protocolRevisionsToUrls[serviceInstance.Protocol]
+			if revisionsToUrls == nil {
+				protocolRevisionsToUrls[serviceInstance.Protocol] = make(map[*gxset.HashSet][]*common.URL)
+				revisionsToUrls = protocolRevisionsToUrls[serviceInstance.Protocol]
+			}
+			urls := revisionsToUrls[revisions]
+			if urls != nil {
+				newServiceURLs[serviceInstance.GetMatchKey()] = urls
+			} else {
+				urls = make([]*common.URL, 0, 8)
+				for _, v := range revisions.Values() {
+					r := v.(string)
+					for _, i := range revisionToInstances[r] {
+						if i != nil {
+							urls = append(urls, i.ToURLs()...)
+						}
+					}
+				}
+				revisionsToUrls[revisions] = urls
+				newServiceURLs[serviceInstance.GetMatchKey()] = urls
+			}
+		}
+		lstn.serviceUrls = newServiceURLs
+
+		for key, notifyListener := range lstn.listeners {
+			urls := lstn.serviceUrls[key]
+			for _, url := range urls {
+				notifyListener.Notify(&registry.ServiceEvent{
+					Action:  remoting.EventTypeAdd,
+					Service: url,
+				})
+			}
+		}
+	}
+	return nil
+}
+
+func (lstn *ServiceInstancesChangedListener) getMetadataInfo(instance registry.ServiceInstance, metadataInfo *common.MetadataInfo, revision string) (*common.MetadataInfo, error) {
+	metadataStorageType := instance.GetMetadata()[constant.METADATA_STORAGE_TYPE_PROPERTY_NAME]

Review comment:
       pls check the value of `instance.GetMetadata()` is not a non-nil map




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307394



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]

Review comment:
       are you sure the return value of instance.GetMetadata() is a non-nil map? If not, pls check its return value `ok`




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305824



##########
File path: config/testdata/consumer_config_withoutProtocol.yml
##########
@@ -1,82 +1,82 @@
-# dubbo client yaml configure file
-
-filter: ""
-
-# client
-request_timeout : "100ms"
-# connect timeout
-connect_timeout : "100ms"
-check: true
-# application config
-application:
-  organization : "ikurento.com"
-  name  : "BDTService"
-  module : "dubbogo user-info client"
-  version : "0.0.1"
-  owner : "ZX"
-  environment : "dev"
-
-registries :
-
-  "hangzhouzk":
-    protocol: "zookeeper"
-    timeout	: "3s"
-    address: "127.0.0.1:2181"
-    username: ""
-    password: ""
-  "shanghaizk":
-    protocol: "zookeeper"
-    timeout	: "3s"
-    address: "127.0.0.1:2182"
-    username: ""
-    password: ""
-
-references:
-  "UserProvider":
-    registry: "hangzhouzk,shanghaizk"
-    filter: ""
-    version: "1.0"
-    group: "as"
-    interface : "com.ikurento.user.UserProvider"
-    url: "dubbo://127.0.0.1:20000/UserProvider"
-    cluster: "failover"
-    methods :
-      - name: "GetUser"
-        retries: 3
-    params:
-      "serviceid":
-        "soa.com.ikurento.user.UserProvider"
-      "forks": 5
-
-shutdown_conf:
-  timeout: 60s
-  step_timeout: 10s
-
-protocol_conf:
-  dubbo:
-    reconnect_interval: 0
-    connection_number: 2
-    heartbeat_period: "5s"
-    session_timeout: "20s"
-    pool_size: 64
-    pool_ttl: 600
-    # gr_pool_size is recommended to be set to [cpu core number] * 100
-    gr_pool_size: 1200
-    # queue_len is recommended to be set to 64 or 128
-    queue_len: 64
-    # queue_number is recommended to be set to gr_pool_size / 20
-    queue_number: 60
-    getty_session_param:
-      compress_encoding: false
-      tcp_no_delay: true
-      tcp_keep_alive: true
-      keep_alive_period: "120s"
-      tcp_r_buf_size: 262144
-      tcp_w_buf_size: 65536
-      pkg_wq_size: 512
-      tcp_read_timeout: "1s"
-      tcp_write_timeout: "5s"
-      wait_timeout: "1s"
-      max_msg_len: 1024
-      session_name: "client"
-
+# dubbo client yaml configure file
+
+filter: ""
+
+# client
+request_timeout : "100ms"
+# connect timeout
+connect_timeout : "100ms"
+check: true
+# application config
+application:
+  organization : "ikurento.com"
+  name  : "BDTService"
+  module : "dubbogo user-info client"
+  version : "0.0.1"
+  owner : "ZX"
+  environment : "dev"
+
+registries :
+
+  "hangzhouzk":
+    protocol: "zookeeper"
+    timeout	: "3s"
+    address: "127.0.0.1:2181"
+    username: ""
+    password: ""
+  "shanghaizk":
+    protocol: "zookeeper"
+    timeout	: "3s"
+    address: "127.0.0.1:2182"
+    username: ""
+    password: ""
+
+references:
+  "UserProvider":
+    registry: "hangzhouzk,shanghaizk"
+    filter: ""
+    version: "1.0"
+    group: "as"
+    interface : "com.ikurento.user.UserProvider"
+    url: "dubbo://127.0.0.1:20000/UserProvider"
+    cluster: "failover"
+    methods :
+      - name: "GetUser"
+        retries: 3
+    params:
+      "serviceid":
+        "soa.com.ikurento.user.UserProvider"
+      "forks": 5
+
+shutdown_conf:
+  timeout: 60s
+  step_timeout: 10s
+
+protocol_conf:
+  dubbo:
+    reconnect_interval: 0
+    connection_number: 2
+    heartbeat_period: "5s"
+    session_timeout: "20s"
+    pool_size: 64
+    pool_ttl: 600
+    # gr_pool_size is recommended to be set to [cpu core number] * 100
+    gr_pool_size: 1200
+    # queue_len is recommended to be set to 64 or 128
+    queue_len: 64
+    # queue_number is recommended to be set to gr_pool_size / 20
+    queue_number: 60
+    getty_session_param:
+      compress_encoding: false
+      tcp_no_delay: true
+      tcp_keep_alive: true
+      keep_alive_period: "120s"
+      tcp_r_buf_size: 262144
+      tcp_w_buf_size: 65536
+      pkg_wq_size: 512
+      tcp_read_timeout: "1s"
+      tcp_write_timeout: "5s"
+      wait_timeout: "1s"
+      max_msg_len: 1024

Review comment:
       wrong examples. pls set it to 16498688




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304268



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (

Review comment:
       pls split this import block.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305148



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),
+	}
+}
+
+func (mi *MetadataInfo) JavaClassName() string {
+	return "org.apache.dubbo.metadata.MetadataInfo"
+}
+
+// CalAndGetRevision is different from Dubbo because golang doesn't support overload
+// so that we could use interface + method name as identifier and ignore the method params

Review comment:
       could = should




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338634



##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -60,13 +60,33 @@ func (m *MetadataServiceProxy) GetExportedURLs(serviceInterface string, group st
 	res := m.invkr.Invoke(context.Background(), inv)
 	if res.Error() != nil {
 		logger.Errorf("could not get the metadata service from remote provider: %v", res.Error())
-		return []interface{}{}, nil
+		return []*common.URL{}, nil
 	}
 
-	urlStrs := res.Result().(*[]interface{})
+	urlStrs := res.Result().([]string)
+	ret := make([]*common.URL, 0, len(urlStrs))
+	for _, v := range urlStrs {
+		tempURL, err := common.NewURL(v)
+		if err != nil {
+			return []*common.URL{}, err
+		}
+		ret = append(ret, tempURL)
+	}
+	return ret, nil
+}
+
+func (m *MetadataServiceProxy) GetExportedServiceURLs() []*common.URL {
+	logger.Error("you should never invoke this implementation")
+	return nil
+}
+
+func (m *MetadataServiceProxy) GetMetadataServiceURL() *common.URL {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305047



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),

Review comment:
       as I know the default value of a  uber atomic bool is false https://github.com/uber-go/atomic/blob/9c79392530c3be3273bd71db21239f2a5146c10c/bool.go#L41. so u need not to set its value again. pls delete this line.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305514



##########
File path: config/config_loader.go
##########
@@ -20,6 +20,7 @@ package config
 import (
 	"flag"
 	"fmt"
+	hessian "github.com/apache/dubbo-go-hessian2"

Review comment:
       pls format this import block.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615337601



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),
+	}
+}
+
+func (mi *MetadataInfo) JavaClassName() string {
+	return "org.apache.dubbo.metadata.MetadataInfo"
+}
+
+// CalAndGetRevision is different from Dubbo because golang doesn't support overload
+// so that we could use interface + method name as identifier and ignore the method params
+// per my understanding, it's enough because Dubbo actually ignore the url params.
+// please refer org.apache.dubbo.common.URL#toParameterString(java.lang.String...)
+func (mi *MetadataInfo) CalAndGetRevision() string {
+	if mi.Revision != "" && mi.reported.Load() {
+		return mi.Revision
+	}
+	if len(mi.Services) == 0 {
+		return "0"
+	}
+	candidates := make([]string, 8)
+
+	for _, s := range mi.Services {
+		sk := s.serviceKey
+		ms := s.url.Methods
+		if len(ms) == 0 {
+			candidates = append(candidates, sk)
+		} else {
+			for _, m := range ms {
+				// methods are part of candidates
+				candidates = append(candidates, sk+constant.KEY_SEPARATOR+m)
+			}
+		}
+
+		// append url params if we need it
+	}
+	sort.Strings(candidates)
+
+	// it's nearly impossible to be overflow
+	res := uint64(0)
+	for _, c := range candidates {
+		res += uint64(crc32.ChecksumIEEE([]byte(c)))
+	}
+	mi.Revision = fmt.Sprint(res)
+	return mi.Revision
+
+}
+
+func (mi *MetadataInfo) HasReported() bool {
+	return mi.reported.Load()
+}
+
+func (mi *MetadataInfo) MarkReported() {
+	mi.reported.CAS(false, true)
+}
+
+func (mi *MetadataInfo) AddService(service *ServiceInfo) {
+	if service == nil {
+		return
+	}
+	mi.Services[service.GetMatchKey()] = service
+}
+
+func (mi *MetadataInfo) RemoveService(service *ServiceInfo) {
+	if service == nil {
+		return
+	}
+	delete(mi.Services, service.matchKey)
+}
+
+type ServiceInfo struct {
+	Name     string            `json:"name"`

Review comment:
       omitempty




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303298



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+

Review comment:
       type  RemoteMetadataServiceCreator func() (remote.RemoteMetadataService, error)
   




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307519



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+			if "0" == revision {
+				logger.Infof("Find instance without valid service metadata: %s", instance.GetHost())
+				continue
+			}
+			subInstances := revisionToInstances[revision]
+			if subInstances == nil {
+				subInstances = make([]registry.ServiceInstance, 8)
+			}
+			revisionToInstances[revision] = append(subInstances, instance)
+			metadataInfo := lstn.revisionToMetadata[revision]

Review comment:
       are u sure the value of revision is not ""? If not, pls check the return value 'ok' of `metadataInfo,ok := lstn.revisionToMetadata[revision]`.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305197



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),
+	}
+}
+
+func (mi *MetadataInfo) JavaClassName() string {
+	return "org.apache.dubbo.metadata.MetadataInfo"
+}
+
+// CalAndGetRevision is different from Dubbo because golang doesn't support overload
+// so that we could use interface + method name as identifier and ignore the method params
+// per my understanding, it's enough because Dubbo actually ignore the url params.

Review comment:
       per my understanding => in my opinion




-- 
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] Patrick0308 commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615399218



##########
File path: common/constant/key.go
##########
@@ -313,11 +313,12 @@ const (
 const (
 	SUBSCRIBED_SERVICE_NAMES_KEY               = "subscribed-services"
 	PROVIDER_BY                                = "provided-by"
-	EXPORTED_SERVICES_REVISION_PROPERTY_NAME   = "dubbo.exported-services.revision"
+	EXPORTED_SERVICES_REVISION_PROPERTY_NAME   = "dubbo.metadata.revision"

Review comment:
       old EXPORTED_SERVICES_REVISION_PROPERTY_NAME not be used




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338542



##########
File path: metadata/service/inmemory/service.go
##########
@@ -220,6 +235,20 @@ func (mts *MetadataService) GetServiceDefinitionByServiceKey(serviceKey string)
 	return v.(string), nil
 }
 
+func (mts *MetadataService) GetMetadataInfo(revision string) (*common.MetadataInfo, error) {

Review comment:
       // nolint

##########
File path: metadata/service/inmemory/service.go
##########
@@ -220,6 +235,20 @@ func (mts *MetadataService) GetServiceDefinitionByServiceKey(serviceKey string)
 	return v.(string), nil
 }
 
+func (mts *MetadataService) GetMetadataInfo(revision string) (*common.MetadataInfo, error) {
+	if revision == "" {
+		return mts.metadataInfo, nil
+	}
+	if mts.metadataInfo.CalAndGetRevision() != revision {
+		return nil, nil
+	}
+	return mts.metadataInfo, nil
+}
+
+func (mts *MetadataService) GetExportedServiceURLs() []*common.URL {

Review comment:
       // nolint

##########
File path: metadata/service/inmemory/service.go
##########
@@ -229,3 +258,11 @@ func (mts *MetadataService) RefreshMetadata(string, string) (bool, error) {
 func (mts *MetadataService) Version() (string, error) {
 	return version, nil
 }
+
+func (mts *MetadataService) GetMetadataServiceURL() *common.URL {

Review comment:
       // nolint

##########
File path: metadata/service/inmemory/service.go
##########
@@ -229,3 +258,11 @@ func (mts *MetadataService) RefreshMetadata(string, string) (bool, error) {
 func (mts *MetadataService) Version() (string, error) {
 	return version, nil
 }
+
+func (mts *MetadataService) GetMetadataServiceURL() *common.URL {
+	return mts.metadataServiceURL
+}
+
+func (mts *MetadataService) SetMetadataServiceURL(url *common.URL) {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303864



##########
File path: common/extension/metadata_service.go
##########
@@ -26,42 +26,27 @@ import (
 )
 
 import (
-	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/common/constant"
 	"github.com/apache/dubbo-go/metadata/service"
 )
 
 var (
-	// there will be two types: local or remote

Review comment:
       // there are two types of metadataService: local or remote




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303127



##########
File path: common/constant/key.go
##########
@@ -313,11 +313,12 @@ const (
 const (
 	SUBSCRIBED_SERVICE_NAMES_KEY               = "subscribed-services"
 	PROVIDER_BY                                = "provided-by"
-	EXPORTED_SERVICES_REVISION_PROPERTY_NAME   = "dubbo.exported-services.revision"
+	EXPORTED_SERVICES_REVISION_PROPERTY_NAME   = "dubbo.metadata.revision"

Review comment:
       why not define another const name 'METADATA_REVISION_PROPERTY_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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307313



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"

Review comment:
       pls 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615339133



##########
File path: registry/service_instance.go
##########
@@ -18,7 +18,12 @@
 package registry
 
 import (
+	"encoding/json"

Review comment:
       split it




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615337641



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`
+
+	reported *atomic.Bool `json:"-"`
+}
+
+func NewMetadataInfWithApp(app string) *MetadataInfo {
+	return NewMetadataInfo(app, "", make(map[string]*ServiceInfo))
+}
+
+func NewMetadataInfo(app string, revision string, services map[string]*ServiceInfo) *MetadataInfo {
+	return &MetadataInfo{
+		App:      app,
+		Revision: revision,
+		Services: services,
+		reported: atomic.NewBool(false),
+	}
+}
+
+func (mi *MetadataInfo) JavaClassName() string {
+	return "org.apache.dubbo.metadata.MetadataInfo"
+}
+
+// CalAndGetRevision is different from Dubbo because golang doesn't support overload
+// so that we could use interface + method name as identifier and ignore the method params
+// per my understanding, it's enough because Dubbo actually ignore the url params.
+// please refer org.apache.dubbo.common.URL#toParameterString(java.lang.String...)
+func (mi *MetadataInfo) CalAndGetRevision() string {
+	if mi.Revision != "" && mi.reported.Load() {
+		return mi.Revision
+	}
+	if len(mi.Services) == 0 {
+		return "0"
+	}
+	candidates := make([]string, 8)
+
+	for _, s := range mi.Services {
+		sk := s.serviceKey
+		ms := s.url.Methods
+		if len(ms) == 0 {
+			candidates = append(candidates, sk)
+		} else {
+			for _, m := range ms {
+				// methods are part of candidates
+				candidates = append(candidates, sk+constant.KEY_SEPARATOR+m)
+			}
+		}
+
+		// append url params if we need it
+	}
+	sort.Strings(candidates)
+
+	// it's nearly impossible to be overflow
+	res := uint64(0)
+	for _, c := range candidates {
+		res += uint64(crc32.ChecksumIEEE([]byte(c)))
+	}
+	mi.Revision = fmt.Sprint(res)
+	return mi.Revision
+
+}
+
+func (mi *MetadataInfo) HasReported() bool {
+	return mi.reported.Load()
+}
+
+func (mi *MetadataInfo) MarkReported() {
+	mi.reported.CAS(false, true)
+}
+
+func (mi *MetadataInfo) AddService(service *ServiceInfo) {
+	if service == nil {
+		return
+	}
+	mi.Services[service.GetMatchKey()] = service
+}
+
+func (mi *MetadataInfo) RemoveService(service *ServiceInfo) {
+	if service == nil {
+		return
+	}
+	delete(mi.Services, service.matchKey)
+}
+
+type ServiceInfo struct {
+	Name     string            `json:"name"`
+	Group    string            `json:"group"`
+	Version  string            `json:"version"`
+	Protocol string            `json:"protocol"`
+	Path     string            `json:"path"`
+	Params   map[string]string `json:"params"`
+
+	serviceKey string `json:"-"`
+	matchKey   string `json:"-"`
+	url        *URL   `json:"-"`
+}
+
+func NewServiceInfoWithUrl(url *URL) *ServiceInfo {
+	service := NewServiceInfo(url.Service(), url.Group(), url.Version(), url.Protocol, url.Path, nil)
+	service.url = url
+	// TODO includeKeys load dynamic
+	p := make(map[string]string, 8)
+	for _, keyInter := range IncludeKeys.Values() {
+		key := keyInter.(string)
+		value := url.GetParam(key, "")
+		if len(value) != 0 {
+			p[key] = value
+		}
+		for _, method := range url.Methods {
+			value = url.GetMethodParam(method, key, "")
+			if len(value) != 0 {
+				p[method+"."+key] = value
+			}
+		}
+	}
+	service.Params = p
+	return service
+}
+
+func NewServiceInfo(name string, group string, version string, protocol string, path string, params map[string]string) *ServiceInfo {

Review comment:
       delete reduntant string




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338387



##########
File path: metadata/report/consul/report.go
##########
@@ -44,6 +44,14 @@ type consulMetadataReport struct {
 	client *consul.Client
 }
 
+func (m *consulMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {
+	panic("implement me")
+}
+
+func (m *consulMetadataReport) PublishAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338631



##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -60,13 +60,33 @@ func (m *MetadataServiceProxy) GetExportedURLs(serviceInterface string, group st
 	res := m.invkr.Invoke(context.Background(), inv)
 	if res.Error() != nil {
 		logger.Errorf("could not get the metadata service from remote provider: %v", res.Error())
-		return []interface{}{}, nil
+		return []*common.URL{}, nil
 	}
 
-	urlStrs := res.Result().(*[]interface{})
+	urlStrs := res.Result().([]string)
+	ret := make([]*common.URL, 0, len(urlStrs))
+	for _, v := range urlStrs {
+		tempURL, err := common.NewURL(v)
+		if err != nil {
+			return []*common.URL{}, err
+		}
+		ret = append(ret, tempURL)
+	}
+	return ret, nil
+}
+
+func (m *MetadataServiceProxy) GetExportedServiceURLs() []*common.URL {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338381



##########
File path: metadata/report/consul/report.go
##########
@@ -44,6 +44,14 @@ type consulMetadataReport struct {
 	client *consul.Client
 }
 
+func (m *consulMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303552



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+
+var (
+	creator func() (remote.RemoteMetadataService, error)

Review comment:
       another question: are you sure the creator do not need a lock to protect read/write? You'd better add a lock for this var for the sake of safe programming.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306785



##########
File path: metadata/service/inmemory/metadata_service_proxy_factory.go
##########
@@ -31,10 +32,30 @@ import (
 )
 
 func init() {
-	factory := service.NewBaseMetadataServiceProxyFactory(createProxy)
-	extension.SetMetadataServiceProxyFactory(local, func() service.MetadataServiceProxyFactory {
-		return factory
+	extension.SetLocalMetadataService(constant.DEFAULT_KEY, GetInMemoryMetadataService)
+	once = &sync.Once{}
+}
+
+var factory service.MetadataServiceProxyFactory
+
+var once *sync.Once
+
+func GetInMemoryMetadataServiceProxyFactory() service.MetadataServiceProxyFactory {
+	once.Do(func() {
+		factory = service.NewBaseMetadataServiceProxyFactory(createProxy)
 	})
+	return factory
+}
+
+var factory service.MetadataServiceProxyFactory

Review comment:
       var (
       once *sync.Once
       factory service.MetadataServiceProxyFactory
   )




-- 
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 merged pull request #1161: Imp: remote service discovery

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


   


-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306749



##########
File path: metadata/service/inmemory/metadata_service_proxy_factory.go
##########
@@ -31,10 +32,30 @@ import (
 )
 
 func init() {
-	factory := service.NewBaseMetadataServiceProxyFactory(createProxy)
-	extension.SetMetadataServiceProxyFactory(local, func() service.MetadataServiceProxyFactory {
-		return factory
+	extension.SetLocalMetadataService(constant.DEFAULT_KEY, GetInMemoryMetadataService)
+	once = &sync.Once{}
+}
+
+var factory service.MetadataServiceProxyFactory

Review comment:
       ```go
   var (
       factory service.MetadataServiceProxyFactory
       once *sync.Once
   )
   ```




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338343



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl
+
+import (
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/metadata/remote"
+	"github.com/apache/dubbo-go/registry"
+	"sync"
+)
+
+import (
+	"go.uber.org/atomic"
+)
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/definition"
+	"github.com/apache/dubbo-go/metadata/identifier"
+	"github.com/apache/dubbo-go/metadata/report/delegate"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+)
+
+// version will be used by Version func
+const (
+	version   = "1.0.0"
+	remoteKey = "remote"
+)
+
+// MetadataService is a implement of metadata service which will delegate the remote metadata report
+// This is singleton
+type RemoteMetadataServiceImpl struct {
+	*inmemory.MetadataService
+	exportedRevision   atomic.String
+	subscribedRevision atomic.String
+	delegateReport     *delegate.MetadataReport
+}
+
+var (
+	metadataServiceOnce               sync.Once
+	remoteMetadataServiceImplInstance remote.RemoteMetadataService
+)
+
+func init() {
+	extension.SetMetadataRemoteService(GetRemoteMetadataService)
+}
+
+// GetRemoteMetadataService will create a new remote MetadataService instance
+func GetRemoteMetadataService() (remote.RemoteMetadataService, error) {
+	var err error
+	metadataServiceOnce.Do(func() {
+		var mr *delegate.MetadataReport
+		mr, err = delegate.NewMetadataReport()
+		if err != nil {
+			return
+		}
+		// it will never return error
+		inms, _ := inmemory.GetInMemoryMetadataService()
+		remoteMetadataServiceImplInstance = &RemoteMetadataServiceImpl{
+			// todo serviceName
+			//BaseMetadataService:     service.NewBaseMetadataService(""),
+			MetadataService: inms.(*inmemory.MetadataService),
+			delegateReport:  mr,
+		}
+	})
+	return remoteMetadataServiceImplInstance, err
+}
+
+// PublishMetadata publish the medata info of service from report
+func (mts *RemoteMetadataServiceImpl) PublishMetadata(service string) {
+	info, err := mts.MetadataService.GetMetadataInfo("")
+	if err != nil {
+		logger.Errorf("GetMetadataInfo error[%v]", err)
+		return
+	}
+	if info.HasReported() {
+		return
+	}
+	id := identifier.NewSubscriberMetadataIdentifier(service, info.CalAndGetRevision())
+	err = mts.delegateReport.PublishAppMetadata(id, info)
+	if err != nil {
+		logger.Errorf("Publishing metadata to error[%v]", err)
+		return
+	}
+	info.MarkReported()
+}
+
+// GetMetadata get the medata info of service from report
+func (mts *RemoteMetadataServiceImpl) GetMetadata(instance registry.ServiceInstance) (*common.MetadataInfo, error) {
+	revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+	id := identifier.NewSubscriberMetadataIdentifier(instance.GetServiceName(), revision)
+	return mts.delegateReport.GetAppMetadata(id)
+}
+
+// PublishServiceDefinition will call remote metadata's StoreProviderMetadata to store url info and service definition
+func (mts *RemoteMetadataServiceImpl) PublishServiceDefinition(url *common.URL) error {
+	interfaceName := url.GetParam(constant.INTERFACE_KEY, "")
+	isGeneric := url.GetParamBool(constant.GENERIC_KEY, false)
+	if common.RoleType(common.PROVIDER).Role() == url.GetParam(constant.SIDE_KEY, "") {
+		if len(interfaceName) > 0 && !isGeneric {
+			sv := common.ServiceMap.GetServiceByServiceKey(url.Protocol, url.ServiceKey())
+			sd := definition.BuildServiceDefinition(*sv, url)
+			id := &identifier.MetadataIdentifier{
+				BaseMetadataIdentifier: identifier.BaseMetadataIdentifier{
+					ServiceInterface: interfaceName,
+					Version:          url.GetParam(constant.VERSION_KEY, ""),
+					Group:            url.GetParam(constant.GROUP_KEY, constant.DUBBO),
+					Side:             url.GetParam(constant.SIDE_KEY, constant.PROVIDER_PROTOCOL),
+				},
+			}
+			mts.delegateReport.StoreProviderMetadata(id, sd)
+			return nil
+		}
+		logger.Errorf("publishProvider interfaceName is empty . providerUrl:%v ", url)
+	} else {

Review comment:
       delete 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338361



##########
File path: metadata/remote/impl/service_test.go
##########
@@ -55,6 +55,14 @@ func (mrf *metadataReportFactory) CreateMetadataReport(*common.URL) report.Metad
 
 type metadataReport struct{}
 
+func (mr metadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       add comment




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304242



##########
File path: common/extension/metadata_service.go
##########
@@ -26,42 +26,27 @@ import (
 )
 
 import (
-	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/common/constant"
 	"github.com/apache/dubbo-go/metadata/service"
 )
 
 var (
-	// there will be two types: local or remote
 	metadataServiceInsMap = make(map[string]func() (service.MetadataService, error), 2)

Review comment:
       pls define a lock for this map for the sake of safe programming




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307810



##########
File path: registry/zookeeper/service_discovery.go
##########
@@ -19,6 +19,7 @@ package zookeeper
 
 import (
 	"fmt"
+	"github.com/apache/dubbo-go/registry/event"

Review comment:
       pls 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615339064



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+			if "0" == revision {
+				logger.Infof("Find instance without valid service metadata: %s", instance.GetHost())
+				continue
+			}
+			subInstances := revisionToInstances[revision]
+			if subInstances == nil {
+				subInstances = make([]registry.ServiceInstance, 8)
+			}
+			revisionToInstances[revision] = append(subInstances, instance)
+			metadataInfo := lstn.revisionToMetadata[revision]
+			if metadataInfo == nil {
+				metadataInfo, err = lstn.getMetadataInfo(instance, metadataInfo, revision)
+				if err != nil {
+					return err
+				}
+			}
+			instance.SetServiceMetadata(metadataInfo)
+			for _, service := range metadataInfo.Services {
+				if localServiceToRevisions[service] == nil {
+					localServiceToRevisions[service] = gxset.NewSet()
+				}
+				localServiceToRevisions[service].Add(revision)
+			}
+
+			newRevisionToMetadata[revision] = metadataInfo
+		}
+		lstn.revisionToMetadata = newRevisionToMetadata
+
+		for serviceInstance, revisions := range localServiceToRevisions {
+			revisionsToUrls := protocolRevisionsToUrls[serviceInstance.Protocol]
+			if revisionsToUrls == nil {
+				protocolRevisionsToUrls[serviceInstance.Protocol] = make(map[*gxset.HashSet][]*common.URL)
+				revisionsToUrls = protocolRevisionsToUrls[serviceInstance.Protocol]
+			}
+			urls := revisionsToUrls[revisions]
+			if urls != nil {
+				newServiceURLs[serviceInstance.GetMatchKey()] = urls
+			} else {
+				urls = make([]*common.URL, 0, 8)
+				for _, v := range revisions.Values() {
+					r := v.(string)
+					for _, i := range revisionToInstances[r] {
+						if i != nil {
+							urls = append(urls, i.ToURLs()...)
+						}
+					}
+				}
+				revisionsToUrls[revisions] = urls
+				newServiceURLs[serviceInstance.GetMatchKey()] = urls
+			}
+		}
+		lstn.serviceUrls = newServiceURLs
+
+		for key, notifyListener := range lstn.listeners {
+			urls := lstn.serviceUrls[key]
+			for _, url := range urls {
+				notifyListener.Notify(&registry.ServiceEvent{
+					Action:  remoting.EventTypeAdd,
+					Service: url,
+				})
+			}
+		}
+	}
+	return nil
+}
+
+func (lstn *ServiceInstancesChangedListener) getMetadataInfo(instance registry.ServiceInstance, metadataInfo *common.MetadataInfo, revision string) (*common.MetadataInfo, error) {
+	metadataStorageType := instance.GetMetadata()[constant.METADATA_STORAGE_TYPE_PROPERTY_NAME]
+	if metadataStorageType == constant.REMOTE_METADATA_STORAGE_TYPE {
+		remoteMetadataServiceImpl, err := extension.GetRemoteMetadataService()
+		if err != nil {
+			return nil, err
+		}
+		metadataInfo, err = remoteMetadataServiceImpl.GetMetadata(instance)
+		if err != nil {
+			return nil, err
+		}
+	} else {

Review comment:
       delete 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307394



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]

Review comment:
       are you sure the return value of instance.GetMetadata() is a non-nil map?




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303956



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (

Review comment:
       pls split this import block.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305848



##########
File path: config/testdata/provider_config_withoutProtocol.yml
##########
@@ -1,80 +1,80 @@
-# dubbo server yaml configure file
-
-filter: ""
-# application config
-application:
-  organization : "ikurento.com"
-  name : "BDTService"
-  module : "dubbogo user-info server"
-  version : "0.0.1"
-  owner : "ZX"
-  environment : "dev"
-
-registries :
-  "hangzhouzk":
-    protocol: "zookeeper"
-    timeout	: "3s"
-    address: "127.0.0.1:2181"
-    username: ""
-    password: ""
-  "shanghaizk":
-    protocol: "zookeeper"
-    timeout	: "3s"
-    address: "127.0.0.1:2182"
-    username: ""
-    password: ""
-
-
-services:
-  "UserProvider":
-    registry: "hangzhouzk,shanghaizk"
-    filter: ""
-    # equivalent to interface of dubbo.xml
-    interface : "com.ikurento.user.UserProvider"
-    loadbalance: "random"
-    version: "1.0"
-    group: "as"
-    warmup: "100"
-    cluster: "failover"
-    methods:
-      - name: "GetUser"
-        retries: 1
-        loadbalance: "random"
-
-protocols:
-    "dubbo":
-      name: "dubbo"
-      # while using dubbo protocol, ip cannot is 127.0.0.1, because client of java-dubbo will get 'connection refuse'
-      ip : "127.0.0.1"
-      port : 20000
-  #-   name: "jsonrpc"
-  #    ip: "127.0.0.1"
-  #    port: 20001
-
-shutdown_conf:
-  timeout: 60s
-  step_timeout: 10s
-
-protocol_conf:
-  dubbo:
-    session_number: 700
-    session_timeout: "20s"
-    # gr_pool_size is recommended to be set to [cpu core number] * 10
-    gr_pool_size: 120
-    # queue_len is recommended to be set to 64 or 128
-    queue_len: 64
-    # queue_number is recommended to be set to gr_pool_size / 20
-    queue_number: 6
-    getty_session_param:
-      compress_encoding: false
-      tcp_no_delay: true
-      tcp_keep_alive: true
-      keep_alive_period: "120s"
-      tcp_r_buf_size: 262144
-      tcp_w_buf_size: 65536
-      pkg_wq_size: 512
-      tcp_read_timeout: "1s"
-      tcp_write_timeout: "5s"
-      wait_timeout: "1s"
-      max_msg_len: 1024
-      session_name: "server"
+# dubbo server yaml configure file
+
+filter: ""
+# application config
+application:
+  organization : "ikurento.com"
+  name : "BDTService"
+  module : "dubbogo user-info server"
+  version : "0.0.1"
+  owner : "ZX"
+  environment : "dev"
+
+registries :
+  "hangzhouzk":
+    protocol: "zookeeper"
+    timeout	: "3s"
+    address: "127.0.0.1:2181"
+    username: ""
+    password: ""
+  "shanghaizk":
+    protocol: "zookeeper"
+    timeout	: "3s"
+    address: "127.0.0.1:2182"
+    username: ""
+    password: ""
+
+
+services:
+  "UserProvider":
+    registry: "hangzhouzk,shanghaizk"
+    filter: ""
+    # equivalent to interface of dubbo.xml
+    interface : "com.ikurento.user.UserProvider"
+    loadbalance: "random"
+    version: "1.0"
+    group: "as"
+    warmup: "100"
+    cluster: "failover"
+    methods:
+      - name: "GetUser"
+        retries: 1
+        loadbalance: "random"
+
+protocols:
+    "dubbo":
+      name: "dubbo"
+      # while using dubbo protocol, ip cannot is 127.0.0.1, because client of java-dubbo will get 'connection refuse'
+      ip : "127.0.0.1"
+      port : 20000
+  #-   name: "jsonrpc"
+  #    ip: "127.0.0.1"
+  #    port: 20001
+
+shutdown_conf:
+  timeout: 60s
+  step_timeout: 10s
+
+protocol_conf:
+  dubbo:
+    session_number: 700
+    session_timeout: "20s"
+    # gr_pool_size is recommended to be set to [cpu core number] * 10
+    gr_pool_size: 120
+    # queue_len is recommended to be set to 64 or 128
+    queue_len: 64
+    # queue_number is recommended to be set to gr_pool_size / 20
+    queue_number: 6
+    getty_session_param:
+      compress_encoding: false
+      tcp_no_delay: true
+      tcp_keep_alive: true
+      keep_alive_period: "120s"
+      tcp_r_buf_size: 262144
+      tcp_w_buf_size: 65536
+      pkg_wq_size: 512
+      tcp_read_timeout: "1s"
+      tcp_write_timeout: "5s"
+      wait_timeout: "1s"
+      max_msg_len: 1024

Review comment:
       pls set it to 16498688




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303799



##########
File path: common/extension/metadata_service.go
##########
@@ -26,42 +26,27 @@ import (
 )
 
 import (
-	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/common/constant"
 	"github.com/apache/dubbo-go/metadata/service"
 )
 
 var (
-	// there will be two types: local or remote

Review comment:
       why delete the remark?




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304456



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,

Review comment:
       pls format this array and add just a const on one line. pls keep the same programming 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307751



##########
File path: registry/service_instance.go
##########
@@ -88,6 +115,63 @@ func (d *DefaultServiceInstance) IsHealthy() bool {
 	return d.Healthy
 }
 
+func (d *DefaultServiceInstance) GetAddress() string {

Review comment:
       pls add comment

##########
File path: registry/service_instance.go
##########
@@ -88,6 +115,63 @@ func (d *DefaultServiceInstance) IsHealthy() bool {
 	return d.Healthy
 }
 
+func (d *DefaultServiceInstance) GetAddress() string {
+	if d.Address != "" {
+		return d.Address
+	}
+	if d.Port <= 0 {
+		d.Address = d.Host
+	} else {
+		d.Address = d.Host + ":" + strconv.Itoa(d.Port)
+	}
+	return d.Address
+}
+
+func (d *DefaultServiceInstance) SetServiceMetadata(m *common.MetadataInfo) {
+	d.ServiceMetadata = m
+}

Review comment:
       pls add comment

##########
File path: registry/service_instance.go
##########
@@ -88,6 +115,63 @@ func (d *DefaultServiceInstance) IsHealthy() bool {
 	return d.Healthy
 }
 
+func (d *DefaultServiceInstance) GetAddress() string {
+	if d.Address != "" {
+		return d.Address
+	}
+	if d.Port <= 0 {
+		d.Address = d.Host
+	} else {
+		d.Address = d.Host + ":" + strconv.Itoa(d.Port)
+	}
+	return d.Address
+}
+
+func (d *DefaultServiceInstance) SetServiceMetadata(m *common.MetadataInfo) {
+	d.ServiceMetadata = m
+}
+
+// ToURLs
+func (d *DefaultServiceInstance) ToURLs() []*common.URL {
+	urls := make([]*common.URL, 0, 8)
+	for _, service := range d.ServiceMetadata.Services {
+		url := common.NewURLWithOptions(common.WithProtocol(service.Protocol),
+			common.WithIp(d.Host), common.WithPort(strconv.Itoa(d.Port)),
+			common.WithMethods(service.GetMethods()), common.WithParams(service.GetParams()))
+		urls = append(urls, url)
+	}
+	return urls
+}
+
+func (d *DefaultServiceInstance) GetEndPoints() []*Endpoint {

Review comment:
       pls add comment

##########
File path: registry/service_instance.go
##########
@@ -88,6 +115,63 @@ func (d *DefaultServiceInstance) IsHealthy() bool {
 	return d.Healthy
 }
 
+func (d *DefaultServiceInstance) GetAddress() string {
+	if d.Address != "" {
+		return d.Address
+	}
+	if d.Port <= 0 {
+		d.Address = d.Host
+	} else {
+		d.Address = d.Host + ":" + strconv.Itoa(d.Port)
+	}
+	return d.Address
+}
+
+func (d *DefaultServiceInstance) SetServiceMetadata(m *common.MetadataInfo) {
+	d.ServiceMetadata = m
+}
+
+// ToURLs
+func (d *DefaultServiceInstance) ToURLs() []*common.URL {
+	urls := make([]*common.URL, 0, 8)
+	for _, service := range d.ServiceMetadata.Services {
+		url := common.NewURLWithOptions(common.WithProtocol(service.Protocol),
+			common.WithIp(d.Host), common.WithPort(strconv.Itoa(d.Port)),
+			common.WithMethods(service.GetMethods()), common.WithParams(service.GetParams()))
+		urls = append(urls, url)
+	}
+	return urls
+}
+
+func (d *DefaultServiceInstance) GetEndPoints() []*Endpoint {
+	rawEndpoints := d.Metadata[constant.SERVICE_INSTANCE_ENDPOINTS]
+	if len(rawEndpoints) == 0 {
+		return nil
+	}
+	var endpoints []*Endpoint
+	err := json.Unmarshal([]byte(rawEndpoints), &endpoints)
+	if err != nil {
+		logger.Errorf("json umarshal rawEndpoints[%s] catch error:%s", rawEndpoints, err.Error())
+		return nil
+	}
+	return endpoints
+}
+
+func (d *DefaultServiceInstance) Copy(endpoint *Endpoint) ServiceInstance {

Review comment:
       pls add comment




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338425



##########
File path: metadata/report/delegate/delegate_report.go
##########
@@ -148,6 +148,16 @@ func NewMetadataReport() (*MetadataReport, error) {
 	return bmr, nil
 }
 
+func (mr *MetadataReport) PublishAppMetadata(identifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // nolint

##########
File path: metadata/report/delegate/delegate_report.go
##########
@@ -148,6 +148,16 @@ func NewMetadataReport() (*MetadataReport, error) {
 	return bmr, nil
 }
 
+func (mr *MetadataReport) PublishAppMetadata(identifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {
+	report := instance.GetMetadataReportInstance()
+	return report.PublishAppMetadata(identifier, info)
+}
+
+func (mr *MetadataReport) GetAppMetadata(identifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       // nolint

##########
File path: metadata/report/delegate/delegate_report.go
##########
@@ -300,3 +305,13 @@ func (mr *MetadataReport) doHandlerMetadataCollection(metadataMap map[*identifie
 	}
 	return false
 }
+
+func (mr *MetadataReport) PublishAppMetadata(identifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338460



##########
File path: metadata/report/delegate/delegate_report.go
##########
@@ -300,3 +305,13 @@ func (mr *MetadataReport) doHandlerMetadataCollection(metadataMap map[*identifie
 	}
 	return false
 }
+
+func (mr *MetadataReport) PublishAppMetadata(identifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {
+	report := instance.GetMetadataReportInstance()
+	return report.PublishAppMetadata(identifier, info)
+}
+
+func (mr *MetadataReport) GetAppMetadata(identifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       // nolint

##########
File path: metadata/report/etcd/report.go
##########
@@ -50,6 +50,16 @@ type etcdMetadataReport struct {
 	root   string
 }
 
+func (e *etcdMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       // nolint

##########
File path: metadata/report/etcd/report.go
##########
@@ -50,6 +50,16 @@ type etcdMetadataReport struct {
 	root   string
 }
 
+func (e *etcdMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {
+	// TODO will implement
+	panic("implement me")
+}
+
+func (e *etcdMetadataReport) PublishAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307666



##########
File path: registry/event/event_publishing_service_discovery.go
##########
@@ -18,15 +18,16 @@
 package event
 
 import (
+	"github.com/apache/dubbo-go/metadata/service/inmemory"

Review comment:
       pls 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307688



##########
File path: registry/event/metadata_service_url_params_customizer.go
##########
@@ -19,6 +19,7 @@ package event
 
 import (
 	"encoding/json"
+	"github.com/apache/dubbo-go/common/extension"

Review comment:
       pls 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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615337881



##########
File path: metadata/identifier/subscribe_metadata_identifier.go
##########
@@ -23,6 +23,21 @@ type SubscriberMetadataIdentifier struct {
 	MetadataIdentifier
 }
 
+func NewSubscriberMetadataIdentifier(application string, revision string) *SubscriberMetadataIdentifier {
+	return &SubscriberMetadataIdentifier{
+		Revision: revision,
+		MetadataIdentifier: MetadataIdentifier{
+			Application: application,
+			BaseMetadataIdentifier: BaseMetadataIdentifier{

Review comment:
       delete default value.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303552



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+
+var (
+	creator func() (remote.RemoteMetadataService, error)

Review comment:
       another question: are you sure the creator do not need a lock to protect read/write?




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304096



##########
File path: common/extension/metadata_service.go
##########
@@ -26,42 +26,27 @@ import (
 )
 
 import (
-	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/common/constant"
 	"github.com/apache/dubbo-go/metadata/service"
 )
 
 var (
-	// there will be two types: local or remote
 	metadataServiceInsMap = make(map[string]func() (service.MetadataService, error), 2)
-	// remoteMetadataService
-	remoteMetadataService service.MetadataService
 )
 
-// SetMetadataService will store the msType => creator pair
-func SetMetadataService(msType string, creator func() (service.MetadataService, error)) {
-	metadataServiceInsMap[msType] = creator
+// SetLocalMetadataService will store the msType => creator pair
+func SetLocalMetadataService(key string, creator func() (service.MetadataService, error)) {
+	metadataServiceInsMap[key] = creator
 }
 
-// GetMetadataService will create a MetadataService instance
-func GetMetadataService(msType string) (service.MetadataService, error) {
-	if creator, ok := metadataServiceInsMap[msType]; ok {
-		return creator()
+// GetMetadataService will create a inmemory MetadataService instance
+func GetLocalMetadataService(key string) (service.MetadataService, error) {
+	if key == "" {
+		key = constant.DEFAULT_KEY
 	}
-	return nil, perrors.New(fmt.Sprintf("could not find the metadata service creator for metadataType: %s, please check whether you have imported relative packages, \n"+
-		"local - github.com/apache/dubbo-go/metadata/service/inmemory, \n"+
-		"remote - github.com/apache/dubbo-go/metadata/service/remote", msType))
-}
-
-// GetRemoteMetadataService will get a RemoteMetadataService instance
-func GetRemoteMetadataService() (service.MetadataService, error) {
-	if remoteMetadataService != nil {
-		return remoteMetadataService, nil
-	}
-	if creator, ok := metadataServiceInsMap["remote"]; ok {
-		var err error
-		remoteMetadataService, err = creator()
-		return remoteMetadataService, err
+	if creator, ok := metadataServiceInsMap[key]; ok {

Review comment:
       this func name is GetLocalMetadataService. However, how do u distinguish the metadata service type in metadataServiceInsMap?




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303298



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+

Review comment:
       type  RemoteMetadataServiceCreator func() (remote.RemoteMetadataService, error)
   var creator RemoteMetadataServiceCreator




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307864



##########
File path: test/integrate/dubbo/go-client/go.mod
##########
@@ -7,7 +7,7 @@ require (
 	//```
 	// RUN test ${PR_ORIGIN_REPO} && go mod edit  -replace=github.com/apache/dubbo-go=github.com/${PR_ORIGIN_REPO}@${PR_ORIGIN_COMMITID} || go get -u github.com/apache/dubbo-go@develop
 	//```
-	github.com/apache/dubbo-go v1.5.5
+	github.com/apache/dubbo-go v1.5.6-rc2.0.20210405033601-abbd985fb785

Review comment:
       github.com/apache/dubbo-go v1.5.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] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306088



##########
File path: doc/apache/release_note.md
##########
@@ -1,11 +1,11 @@
-### How to release a new version?
----
-
-* 1 Check the time range of NOTICE;
-* 2 Add the features to the feature list of README.md/README_CN.md/CHANGE.md;
-* 3 Check whether every code file has the Apache License 2.0 or not;
-* 4 There should not be author info(name & email etc) exist in code file;
-* 5 Run all unit tests;
-* 6 Run all samples in apache/dubbo-samples/golang;
-* 7 Write "What's New" by releaser who should be an apache/dubbo-go committer;
+### How to release a new version?
+---
+
+* 1 Check the time range of NOTICE;

Review comment:
       * 2 Pls check the value of Version in common/constant.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338673



##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -60,13 +60,33 @@ func (m *MetadataServiceProxy) GetExportedURLs(serviceInterface string, group st
 	res := m.invkr.Invoke(context.Background(), inv)
 	if res.Error() != nil {
 		logger.Errorf("could not get the metadata service from remote provider: %v", res.Error())
-		return []interface{}{}, nil
+		return []*common.URL{}, nil
 	}
 
-	urlStrs := res.Result().(*[]interface{})
+	urlStrs := res.Result().([]string)
+	ret := make([]*common.URL, 0, len(urlStrs))
+	for _, v := range urlStrs {
+		tempURL, err := common.NewURL(v)
+		if err != nil {
+			return []*common.URL{}, err
+		}
+		ret = append(ret, tempURL)
+	}
+	return ret, nil
+}
+
+func (m *MetadataServiceProxy) GetExportedServiceURLs() []*common.URL {
+	logger.Error("you should never invoke this implementation")
+	return nil
+}
+
+func (m *MetadataServiceProxy) GetMetadataServiceURL() *common.URL {

Review comment:
       // todo

##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -60,13 +60,33 @@ func (m *MetadataServiceProxy) GetExportedURLs(serviceInterface string, group st
 	res := m.invkr.Invoke(context.Background(), inv)
 	if res.Error() != nil {
 		logger.Errorf("could not get the metadata service from remote provider: %v", res.Error())
-		return []interface{}{}, nil
+		return []*common.URL{}, nil
 	}
 
-	urlStrs := res.Result().(*[]interface{})
+	urlStrs := res.Result().([]string)
+	ret := make([]*common.URL, 0, len(urlStrs))
+	for _, v := range urlStrs {
+		tempURL, err := common.NewURL(v)
+		if err != nil {
+			return []*common.URL{}, err
+		}
+		ret = append(ret, tempURL)
+	}
+	return ret, nil
+}
+
+func (m *MetadataServiceProxy) GetExportedServiceURLs() []*common.URL {

Review comment:
       // todo

##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -60,13 +60,33 @@ func (m *MetadataServiceProxy) GetExportedURLs(serviceInterface string, group st
 	res := m.invkr.Invoke(context.Background(), inv)
 	if res.Error() != nil {
 		logger.Errorf("could not get the metadata service from remote provider: %v", res.Error())
-		return []interface{}{}, nil
+		return []*common.URL{}, nil
 	}
 
-	urlStrs := res.Result().(*[]interface{})
+	urlStrs := res.Result().([]string)
+	ret := make([]*common.URL, 0, len(urlStrs))
+	for _, v := range urlStrs {
+		tempURL, err := common.NewURL(v)
+		if err != nil {
+			return []*common.URL{}, err
+		}
+		ret = append(ret, tempURL)
+	}
+	return ret, nil
+}
+
+func (m *MetadataServiceProxy) GetExportedServiceURLs() []*common.URL {
+	logger.Error("you should never invoke this implementation")
+	return nil
+}
+
+func (m *MetadataServiceProxy) GetMetadataServiceURL() *common.URL {
+	logger.Error("you should never invoke this implementation")
+	return nil
+}
 
-	ret := make([]interface{}, 0, len(*urlStrs))
-	return append(ret, *urlStrs...), nil
+func (m *MetadataServiceProxy) SetMetadataServiceURL(*common.URL) {

Review comment:
       // todo

##########
File path: metadata/service/inmemory/service_proxy.go
##########
@@ -132,3 +152,20 @@ func (m *MetadataServiceProxy) Version() (string, error) {
 	logger.Error("you should never invoke this implementation")
 	return "", nil
 }
+
+func (m *MetadataServiceProxy) GetMetadataInfo(revision string) (*common.MetadataInfo, error) {

Review comment:
       // todo




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303621



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+
+var (
+	creator func() (remote.RemoteMetadataService, error)
+)
+
+// SetMetadataRemoteService will store the

Review comment:
       pls add a complete remark




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338362



##########
File path: metadata/remote/impl/service_test.go
##########
@@ -55,6 +55,14 @@ func (mrf *metadataReportFactory) CreateMetadataReport(*common.URL) report.Metad
 
 type metadataReport struct{}
 
+func (mr metadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {
+	panic("implement me")
+}
+
+func (mr metadataReport) PublishAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304757



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`

Review comment:
       ```go
   type MetadataInfo struct {
   	App      string `json:"app,omitempty"`
   	Revision string `json:"revision,omitempty"`
            Services map[string]*ServiceInfo `json:"services"`
   ```




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615307582



##########
File path: registry/event/event_listener.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 event
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+	"github.com/apache/dubbo-go/registry"
+	"github.com/apache/dubbo-go/remoting"
+	gxset "github.com/dubbogo/gost/container/set"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/observer"
+)
+
+// The Service Discovery Changed  Event Listener
+type ServiceInstancesChangedListener struct {
+	registry.ServiceInstancesChangedListenerBase
+	ServiceNames       *gxset.HashSet
+	listeners          map[string]registry.NotifyListener
+	serviceUrls        map[string][]*common.URL
+	revisionToMetadata map[string]*common.MetadataInfo
+	allInstances       map[string][]registry.ServiceInstance
+}
+
+func NewServiceInstancesChangedListener(services *gxset.HashSet) *ServiceInstancesChangedListener {
+	return &ServiceInstancesChangedListener{
+		ServiceNames:       services,
+		listeners:          make(map[string]registry.NotifyListener),
+		serviceUrls:        make(map[string][]*common.URL),
+		revisionToMetadata: make(map[string]*common.MetadataInfo),
+		allInstances:       make(map[string][]registry.ServiceInstance),
+	}
+}
+
+// OnEvent on ServiceInstancesChangedEvent the service instances change event
+func (lstn *ServiceInstancesChangedListener) OnEvent(e observer.Event) error {
+	ce, ok := e.(*registry.ServiceInstancesChangedEvent)
+	if !ok {
+		return nil
+	}
+	var err error
+	lstn.allInstances[ce.ServiceName] = ce.Instances
+	revisionToInstances := make(map[string][]registry.ServiceInstance)
+	newRevisionToMetadata := make(map[string]*common.MetadataInfo)
+	localServiceToRevisions := make(map[*common.ServiceInfo]*gxset.HashSet)
+	protocolRevisionsToUrls := make(map[string]map[*gxset.HashSet][]*common.URL)
+	newServiceURLs := make(map[string][]*common.URL)
+
+	for _, instances := range lstn.allInstances {
+		for _, instance := range instances {
+			revision := instance.GetMetadata()[constant.EXPORTED_SERVICES_REVISION_PROPERTY_NAME]
+			if "0" == revision {
+				logger.Infof("Find instance without valid service metadata: %s", instance.GetHost())
+				continue
+			}
+			subInstances := revisionToInstances[revision]
+			if subInstances == nil {
+				subInstances = make([]registry.ServiceInstance, 8)
+			}
+			revisionToInstances[revision] = append(subInstances, instance)
+			metadataInfo := lstn.revisionToMetadata[revision]
+			if metadataInfo == nil {
+				metadataInfo, err = lstn.getMetadataInfo(instance, metadataInfo, revision)
+				if err != nil {
+					return err
+				}
+			}
+			instance.SetServiceMetadata(metadataInfo)
+			for _, service := range metadataInfo.Services {
+				if localServiceToRevisions[service] == nil {
+					localServiceToRevisions[service] = gxset.NewSet()
+				}
+				localServiceToRevisions[service].Add(revision)
+			}
+
+			newRevisionToMetadata[revision] = metadataInfo
+		}
+		lstn.revisionToMetadata = newRevisionToMetadata
+
+		for serviceInstance, revisions := range localServiceToRevisions {
+			revisionsToUrls := protocolRevisionsToUrls[serviceInstance.Protocol]
+			if revisionsToUrls == nil {
+				protocolRevisionsToUrls[serviceInstance.Protocol] = make(map[*gxset.HashSet][]*common.URL)
+				revisionsToUrls = protocolRevisionsToUrls[serviceInstance.Protocol]
+			}
+			urls := revisionsToUrls[revisions]

Review comment:
       the same as above. pls check `ok`




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615305917



##########
File path: metadata/identifier/subscribe_metadata_identifier.go
##########
@@ -32,3 +47,18 @@ func (mdi *SubscriberMetadataIdentifier) GetIdentifierKey() string {
 func (mdi *SubscriberMetadataIdentifier) GetFilePathKey() string {
 	return mdi.BaseMetadataIdentifier.getFilePathKey(mdi.Revision)
 }
+
+func NewSubscriberMetadataIdentifier(application string, revision string) *SubscriberMetadataIdentifier {
+	return &SubscriberMetadataIdentifier{
+		Revision: revision,
+		MetadataIdentifier: MetadataIdentifier{
+			Application: application,
+			BaseMetadataIdentifier: BaseMetadataIdentifier{
+				ServiceInterface: "",

Review comment:
       hey, guy, pls remember that, if its default value is "", u need not to assign the same value to it again.




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615303436



##########
File path: common/extension/metadata_remote.go
##########
@@ -0,0 +1,25 @@
+package extension
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/metadata/remote"
+	perrors "github.com/pkg/errors"
+)
+
+var (
+	creator func() (remote.RemoteMetadataService, error)
+)
+
+// SetMetadataRemoteService will store the
+func SetMetadataRemoteService(creatorFunc func() (remote.RemoteMetadataService, error)) {

Review comment:
       func SetMetadataRemoteService(creatorFunc RemoteMetadataServiceCreator) {




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615338473



##########
File path: metadata/report/nacos/report.go
##########
@@ -50,6 +50,16 @@ type nacosMetadataReport struct {
 	client config_client.IConfigClient
 }
 
+func (n *nacosMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {

Review comment:
       // nolint

##########
File path: metadata/report/nacos/report.go
##########
@@ -50,6 +50,16 @@ type nacosMetadataReport struct {
 	client config_client.IConfigClient
 }
 
+func (n *nacosMetadataReport) GetAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier) (*common.MetadataInfo, error) {
+	// TODO will implement
+	panic("implement me")
+}
+
+func (n *nacosMetadataReport) PublishAppMetadata(metadataIdentifier *identifier.SubscriberMetadataIdentifier, info *common.MetadataInfo) error {

Review comment:
       // 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


[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615304757



##########
File path: common/metadata_info.go
##########
@@ -0,0 +1,221 @@
+/*
+ * 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 common
+
+import (
+	"fmt"
+	"github.com/apache/dubbo-go/common/constant"
+	gxset "github.com/dubbogo/gost/container/set"
+	"go.uber.org/atomic"
+	"hash/crc32"
+	"net/url"
+	"sort"
+	"strings"
+)
+
+var IncludeKeys = gxset.NewSet(
+	constant.APPLICATION_KEY,
+	constant.GROUP_KEY, constant.TIMESTAMP_KEY, constant.SERIALIZATION_KEY, constant.CLUSTER_KEY,
+	constant.LOADBALANCE_KEY, constant.PATH_KEY, constant.TIMEOUT_KEY,
+	constant.TOKEN_KEY, constant.VERSION_KEY, constant.WARMUP_KEY,
+	constant.WEIGHT_KEY, constant.RELEASE_KEY)
+
+type MetadataInfo struct {
+	App      string                  `json:"app"`
+	Revision string                  `json:"revision"`
+	Services map[string]*ServiceInfo `json:"services"`

Review comment:
       type MetadataInfo struct {
   	App      string `json:"app,omitempty"`
   	Revision string `json:"revision,omitempty"`
            Services map[string]*ServiceInfo `json:"services"`
   }




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306537



##########
File path: metadata/remote/impl/service.go
##########
@@ -0,0 +1,151 @@
+/*
+ * 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 impl
+
+import (
+	"github.com/apache/dubbo-go/common/extension"
+	"github.com/apache/dubbo-go/metadata/remote"
+	"github.com/apache/dubbo-go/registry"
+	"sync"
+)
+
+import (
+	"go.uber.org/atomic"
+)
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/metadata/definition"
+	"github.com/apache/dubbo-go/metadata/identifier"
+	"github.com/apache/dubbo-go/metadata/report/delegate"
+	"github.com/apache/dubbo-go/metadata/service/inmemory"
+)
+
+// version will be used by Version func
+const (
+	version   = "1.0.0"
+	remoteKey = "remote"
+)
+
+// MetadataService is a implement of metadata service which will delegate the remote metadata report
+// This is singleton
+type RemoteMetadataServiceImpl struct {
+	*inmemory.MetadataService
+	exportedRevision   atomic.String
+	subscribedRevision atomic.String
+	delegateReport     *delegate.MetadataReport
+}
+
+var (
+	metadataServiceOnce               sync.Once
+	remoteMetadataServiceImplInstance remote.RemoteMetadataService
+)
+
+func init() {
+	extension.SetMetadataRemoteService(GetRemoteMetadataService)
+}
+
+// GetRemoteMetadataService will create a new remote MetadataService instance
+func GetRemoteMetadataService() (remote.RemoteMetadataService, error) {
+	var err error
+	metadataServiceOnce.Do(func() {
+		var mr *delegate.MetadataReport
+		mr, err = delegate.NewMetadataReport()
+		if err != nil {
+			return
+		}
+		// it will never return error
+		inms, _ := inmemory.GetInMemoryMetadataService()
+		remoteMetadataServiceImplInstance = &RemoteMetadataServiceImpl{
+			// todo serviceName
+			//BaseMetadataService:     service.NewBaseMetadataService(""),
+			MetadataService: inms.(*inmemory.MetadataService),
+			delegateReport:  mr,
+		}
+	})
+	return remoteMetadataServiceImplInstance, err
+}
+
+// PublishMetadata publish the medata info of service from report

Review comment:
       // PublishMetadata publishes the metadata info of @service to remote metadata center




-- 
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 #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615306957



##########
File path: registry/consul/service_discovery.go
##########
@@ -20,6 +20,7 @@ package consul
 import (
 	"encoding/base64"
 	"fmt"
+	"github.com/apache/dubbo-go/registry/event"

Review comment:
       move it to another import block.




-- 
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] Patrick0308 commented on a change in pull request #1161: Imp: remote service discovery

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #1161:
URL: https://github.com/apache/dubbo-go/pull/1161#discussion_r615399022



##########
File path: registry/event/base_configuration_listener.go
##########
@@ -0,0 +1,118 @@
+/*
+ * 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 event
+
+import (
+	perrors "github.com/pkg/errors"
+)
+
+import (
+	"github.com/apache/dubbo-go/common"
+	"github.com/apache/dubbo-go/common/config"
+	"github.com/apache/dubbo-go/common/constant"
+	"github.com/apache/dubbo-go/common/logger"
+	"github.com/apache/dubbo-go/config_center"
+	"github.com/apache/dubbo-go/remoting"
+)
+
+// nolint
+type BaseConfigurationListener struct {
+	configurators           []config_center.Configurator
+	dynamicConfiguration    config_center.DynamicConfiguration
+	defaultConfiguratorFunc func(url *common.URL) config_center.Configurator
+}
+
+// Configurators gets Configurator from config center
+func (bcl *BaseConfigurationListener) Configurators() []config_center.Configurator {
+	return bcl.configurators
+}
+
+// InitWith will init BaseConfigurationListener by @key+@Listener+@f
+func (bcl *BaseConfigurationListener) InitWith(key string, listener config_center.ConfigurationListener,
+	f func(url *common.URL) config_center.Configurator) {
+
+	bcl.dynamicConfiguration = config.GetEnvInstance().GetDynamicConfiguration()
+	if bcl.dynamicConfiguration == nil {
+		// set configurators to empty
+		bcl.configurators = []config_center.Configurator{}
+		return
+	}
+	bcl.defaultConfiguratorFunc = f
+	bcl.dynamicConfiguration.AddListener(key, listener)
+	if rawConfig, err := bcl.dynamicConfiguration.GetInternalProperty(key,
+		config_center.WithGroup(constant.DUBBO)); err != nil {
+		// set configurators to empty
+		bcl.configurators = []config_center.Configurator{}
+		return
+	} else if len(rawConfig) > 0 {
+		if err := bcl.genConfiguratorFromRawRule(rawConfig); err != nil {
+			logger.Error("bcl.genConfiguratorFromRawRule(rawConfig:%v) = error:%v", rawConfig, err)
+		}
+	}
+}
+
+// Process the notification event once there's any change happens on the config.
+func (bcl *BaseConfigurationListener) Process(event *config_center.ConfigChangeEvent) {
+	logger.Infof("Notification of overriding rule, change type is: %v , raw config content is:%v", event.ConfigType, event.Value)

Review comment:
       I change Infof to Debugf




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