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

nacos-client中endpoint参数和serverAddr优先级问题 #12218

Closed
misakacoder opened this issue Jun 14, 2024 · 18 comments
Closed

nacos-client中endpoint参数和serverAddr优先级问题 #12218

misakacoder opened this issue Jun 14, 2024 · 18 comments
Labels
area/Client Related to Nacos Client SDK area/Naming kind/bug Category issues or prs related to bug.
Milestone

Comments

@misakacoder
Copy link
Contributor

分支:develop

这是官网对于通用参数的说明:

image

这是代码运行结果:

image

@kangzhaok
Copy link
Contributor

image
endpoint并不是真实的nacos地址,相当于一个地址管理的存在

@misakacoder
Copy link
Contributor Author

@kangzhaok endpoint配置的是地址服务器,client会定时去读地址服务器来获取和维护nacos集群列表,这样比较灵活

@misakacoder
Copy link
Contributor Author

@kangzhaok 但是他这个文档说的直接配置serverAddr比endpoint读取nacos地址的优先级高,但是我看代码好像并不是这样

@kangzhaok
Copy link
Contributor

@kangzhaok 但是他这个文档说的直接配置serverAddr比endpoint读取nacos地址的优先级高,但是我看代码好像并不是这样

是的,nacos默认的nacos.use.endpiont.parsing.rule是true

@KomachiSion
Copy link
Collaborator

应该是期间重构的时候把顺序搞错了, 可能需要修复下。

不过这个逻辑修改了很长时间了, 可能需要重新评估到底哪个优先级应该更高了。

@KomachiSion KomachiSion added the kind/discussion Category issues related to discussion label Jun 17, 2024
@misakacoder
Copy link
Contributor Author

@KomachiSion 如果设置isUseEndpointParsingRule为false,config的ServerListManager的地址优先级就是对的,但是我看默认值是true

我那个pr那行代码那里

image

这里会把serverAddrsStr置空,构造函数又是优先判断serverAddrsStr字段的,这样看isUseEndpointParsingRule也会影响优先级了

image

@KomachiSion
Copy link
Collaborator

那应该把注册中心的也改成serveraddr优先,应该是注册中心的逻辑在重构中改变了,欢迎PR修复一下,我理解只要把判断顺序调换一下即可。

@KomachiSion KomachiSion added area/Client Related to Nacos Client SDK area/Naming kind/bug Category issues or prs related to bug. and removed kind/discussion Category issues related to discussion labels Jun 24, 2024
@KomachiSion KomachiSion added this to the 2.4.0 milestone Jun 24, 2024
@misakacoder
Copy link
Contributor Author

@KomachiSion 就是都改成按文档说的serverAddr优先呗,但是这个也受天池大赛的影响吧,比赛可以一起重构了?

@KomachiSion
Copy link
Collaborator

@KomachiSion 就是都改成按文档说的serverAddr优先呗,但是这个也受天池大赛的影响吧,比赛可以一起重构了?

先稍等一下, 我梳理了一下从1.1.4到目前版本下优先级的内容,似乎只有少数场景(配置中心,parsing=false)时才是serverAddr优先。也就是大多数情况下应该是endpoint优先。

这里还需要在讨论一下, 看下是该文档还是该代码。

@misakacoder
Copy link
Contributor Author

@KomachiSion 配置中心parsing=false时,serverAddr优先这个应该是bug吧,parsing应该只影响endpoint才对,不应该影响别的参数。当然代码优先级和文档不符这个,确实endpoint优先好一点,实际上使用endpoint比serverAddr灵活一些。

@KomachiSion
Copy link
Collaborator

@KomachiSion 配置中心parsing=false时,serverAddr优先这个应该是bug吧,parsing应该只影响endpoint才对,不应该影响别的参数。当然代码优先级和文档不符这个,确实endpoint优先好一点,实际上使用endpoint比serverAddr灵活一些。

根据历史版本和多数使用场景的行为,发现绝大多数情况和场景下,都是endpoint优先, 仅在配置中心的parsing=false时优先,且分析应该目前没有parsing=false且同时配置endpoint和serverAddr的诉求。

因此最终预期,统一成endpoint优先:

  1. parsing=true(默认)情况下,保持现状,即:最 高优先级环境变变量 ALIBABA_ALIWARE_ENDPOINT_URL -> 其次是 用户传入的endpoing-Dendpoint -> 最后是传入的serverAddr-DserverAddr.
  2. parsing=false情况下, 忽略ALIBABA_ALIWARE_ENDPOINT_URL, 其他保持和parsing=true一致, 即 用户传入的endpoing-Dendpoint -> 传入的serverAddr-DserverAddr.

文档可能需要修改说明下。

@misakacoder
Copy link
Contributor Author

  1. parsing=false情况下, 忽略ALIBABA_ALIWARE_ENDPOINT_URL, 其他保持和parsing=true一致, 即 用户传入的endpoing-Dendpoint -> 传入的serverAddr-DserverAddr.

我理解的parsing就只是用来控制endpoint是否需要当成变量来解析,就算设置成false,环境变量优先级高于命令行参数或者Properties还是应该存在的吧,而不是忽略环境变量。

@KomachiSion
Copy link
Collaborator

  1. parsing=false情况下, 忽略ALIBABA_ALIWARE_ENDPOINT_URL, 其他保持和parsing=true一致, 即 用户传入的endpoing-Dendpoint -> 传入的serverAddr-DserverAddr.

我理解的parsing就只是用来控制endpoint是否需要当成变量来解析,就算设置成false,环境变量优先级高于命令行参数或者Properties还是应该存在的吧,而不是忽略环境变量。

忽略 ALIBABA_ALIWARE_ENDPOINT_URL,可以查看现在的代码,只有parsing=true的情况,才会进入到ParamUtil.parsingEndpointRule去读取环境变量, 否则只会获取endpoint参数

可以这么认为parsing的开关就是是否解析ALIBABA_ALIWARE_ENDPOINT_URL和endpoing中的变量替换的开关。

如果parsing为false, 其实就只根据endpoint参数和serverAddr参数指定的值进行连接,不考虑其中是否有变量,不考虑是否有特殊配置,

@misakacoder
Copy link
Contributor Author

同时设置env和prop

1. parsing=true,优先读取env

image

2. parsing=false,优先读取prop

image

3.parsing=true,env为变量,prop为常量,最终读取了env但是未解析成常量

image

4.parsing=false,env为变量,prop为常量,最终读取了prop

image

5.parsing=true,env为常量,prop为变量,最终读取了prop并解析成常量

image

6.parsing=false,env为常量,prop为变量,最终读取了prop

image

@KomachiSion 以上测试结果3和5和你描述的也还是有些不一样,env和prop都为变量的情况就没测试了,看不出来优先读取的是谁。其实我理解的这个设计,env传参、(-D、prop)传参 (nacos 这2个类型好像没有优先级区别) 应该是类似于SpringBoot的配置优先级一样来设计的吧,相同的key的配置,-D > env > yml,但是测试出来又不完全是,不知道是不是按照我想的这样设计的。

@KomachiSion
Copy link
Collaborator

你的运行结果3就是符合预期的,生效的就是ALIBABA_ALIWARE_ENDPOINT_URL,ALIBABA_ALIWARE_ENDPOINT_URL不一定是变量,直接的地址也是可以的。

prop包含了两部分,用户传的prop和-D, 这里统称为prop可以, 本issue主要讨论的是prop中同时存在serverAddr和endpoint时的结果,不考虑prop是-D优先还是传统prop优先, 这个方面可以读文档。

@KomachiSion
Copy link
Collaborator

KomachiSion commented Jul 3, 2024

另外5的话, 确实如果endpoint有变量时,优先级高于ALIBABA_ALIWARE_ENDPOINT_URL, 但是如果两者都没有变量时,ALIBABA_ALIWARE_ENDPOINT_URL优先级更高。

这个部分逻辑反正封装在ParamUtil.parsingEndpointRule中,只要保持这里面逻辑保持一致即可。

@misakacoder
Copy link
Contributor Author

另外5的话, 确实如果endpoint有变量时,优先级高于ALIBABA_ALIWARE_ENDPOINT_URL, 但是如果两者都没有变量时,ALIBABA_ALIWARE_ENDPOINT_URL优先级更高。

这个部分逻辑反正封装在ParamUtil.parsingEndpointRule中,只要保持这里面逻辑保持一致即可。

嗯,因为我理解的是env > prop,即先从环境变量中找ALIBABA_ALIWARE_ENDPOINT_URL的值,如果值为空,再从prop中找endpoint的值,最后按照parsing的值来确实是否需要解析前面找到的值为最终的endpoint。反正重构寻址保持原来的endpoint解析的逻辑就行了呗。实际上用的话,估计也配不到我上面这么多种复杂的组合。

@KomachiSion KomachiSion modified the milestones: 2.4.0, 2.4.1 Jul 24, 2024
@KomachiSion KomachiSion modified the milestones: 2.4.1, 2.4.2 Aug 19, 2024
@KomachiSion KomachiSion removed this from the 2.4.2 milestone Sep 6, 2024
@KomachiSion KomachiSion added this to the 2.5.0 milestone Sep 6, 2024
@KomachiSion
Copy link
Collaborator

Fixed by #12189

@KomachiSion KomachiSion closed this as not planned Won't fix, can't repro, duplicate, stale Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/Client Related to Nacos Client SDK area/Naming kind/bug Category issues or prs related to bug.
Projects
None yet
Development

No branches or pull requests

3 participants