Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

2.3.0-BETA服务端检查批量注册接口batchRegisterInstance参数serviceName的正则规则不支持nacos服务查询应答结果里各个instance对象的serviceName #11298

Closed
zrlw opened this issue Oct 28, 2023 · 9 comments · Fixed by #11342

Comments

@zrlw
Copy link
Contributor

zrlw commented Oct 28, 2023

Describe the bug
2.3.0-BETA服务端检查批量注册接口batchRegisterInstance参数serviceName的正则规则在ParamCheckRule.java定义:

public String serviceNamePatternString = "^(?!@).((?!@@)[^\\u4E00-\\u9FA5\\s])*$";

serviceNamePatternString正则表达式不支持@,但是我们的nacos2服务端返回的service查询实例对象里的instances实例列表的各个instance的serviceName是包含组名的:“DEFAULT_GROUP@@servicename”,服务名里面都有连续的两个“@”字符。

nacos-sync同步nacos注册中心A集群的实例到nacos注册中心B集群遇到的问题:通过service查询获取A集群的全部instance之后,要将各个instance对象里serviceName里的组名前缀删除或者将serviceName属性重置为null,才能调用B集群的批量注册接口,否则B集群对instance的serviceName参数进行检查时就会失败。

@KomachiSion
Copy link
Collaborator

正则禁止以@开头,连续的@@字符,和中文字符, 其他字符不做限制。

@zrlw
Copy link
Contributor Author

zrlw commented Oct 30, 2023

订正一下,正则表达式的确只是限制@字符,我复制表达式到测试工具时忘了把双反斜杠改成单反斜杠。
这个问题实际原因是nacos查询服务实例返回的各个instance的serviceName属性是包含“组名@@”前缀的服务名称,不符合nacos2新增的临时实例批量注册接口要求的serviceName格式。

@zrlw zrlw changed the title 2.3.0-BETA服务端检查批量注册接口batchRegisterInstance参数serviceName的正则规则不支持dubbo接口级别实例名称 2.3.0-BETA服务端检查批量注册接口batchRegisterInstance参数serviceName的正则规则不支持nacos服务查询应答结果里各个instance对象的serviceName Oct 30, 2023
@KomachiSion
Copy link
Collaborator

我记得是有处理的, 会把组名@@服务名处理成 组名 和 服务名后进行处理。

@zrlw
Copy link
Contributor Author

zrlw commented Nov 6, 2023

这个问题是我们搞nacos-sync兼容nacos2.x批量注册PR时遇到的,nacos-sync负责将相关服务实例从nacos源集群同步到目标集群。
nacos-sync调用nacosNamingService#getAllInstances(serviceName, groupName, new ArrayList<>(), true)从nacos源集群获取临时实例instances列表,我们发现instances列表里的每个instance对象里的serviceName属性都包含组名@@前缀,如果把源集群返回的instances列表作为nacosNamingService#batchRegisterInstance(String serviceName, String groupName, List<Instance> instances)的入参,nacos-B集群接收批量注册请求的节点就会报serviceName参数非法,我们只好加了个预处理,将每个instance的serviceName置null(去掉组名前缀应该也可以)之后再去调用批量注册接口。
我们测试的nacos集群版本是2.3.0-BETA。

顺便问一下nacos-sync社区是不是没人维护了?issue和pr已经有小半年没有处理了,我们想推进nacos-sync支持nacos2临时实例批量同步,以及consul、eureka、zookeeper和nacos2.x版本集群之间的同步功能,需要怎么做?

@zrlw
Copy link
Contributor Author

zrlw commented Nov 6, 2023

nacos-sync支持nacos2.x临时实例批量注册方法的issue
nacos-group/nacos-sync#339
nacos-sync支持nacos2.x临时实例批量注册方法的PR
nacos-group/nacos-sync#336

@zrlw
Copy link
Contributor Author

zrlw commented Nov 7, 2023

其实还有一个客户端redo任务服务名匹配问题,当注册临时实例请求参数未设置instance对象的instanceId、serviceName时,nacos客户端redoTask保存的实例对象里的instanceId、serviceName均为null;
当要解除该注册实例时,调用NamingGrpcClientProxy#batchDeregisterInstance方法的instance入参对象里的instanceId、serviceName属性均要设为null,如果取nacos服务端返回的instance对象的instanceId、serviceName,无论serviceName是否截除组名@@前缀,都解除不了实例:
因为batchDeregisterInstance要调用getRetainInstance判断缓存的临时实例中是否需要保留在注册中心上的语句是!instanceMap.containsKey(instance),而Instance对象equals是Instance.toString()的字符串比较,Instance#toString()方法包括instanceId、serviceName属性,instanceId或serviceName属性非null的Instance对象与redoTask保留的instanceId、serviceName均为null的Instance对象比较的结果恒为false,导致getRetainInstance的处理结果总是要保留所有实例。
getRetainInstance这个方法containsKey比较条件应该改成只比较ip和port。

@zrlw
Copy link
Contributor Author

zrlw commented Nov 7, 2023

服务端收到grpc注册请求时,会先执行RemoteParamCheckFilter调用DefaultParamChecker检查入参,批量注册接口入参的instances里的serviceName也会被查,简单起见,在客户端请求时自动剔出组名前缀应该就行了。

@KomachiSion
Copy link
Collaborator

所以应该就差一个betchRegister的时候有问题了对吧。

@zrlw
Copy link
Contributor Author

zrlw commented Nov 9, 2023

所以应该就差一个betchRegister的时候有问题了对吧。

主要是这个问题,另外客户端解除注册实例请求时比较redotask缓存实例的判断逻辑也有问题,我分开提两个PR吧。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants