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

Spi重复初始化资源问题 #3376

Closed
ZShUn opened this issue Apr 18, 2024 · 9 comments · Fixed by #3387
Closed

Spi重复初始化资源问题 #3376

ZShUn opened this issue Apr 18, 2024 · 9 comments · Fixed by #3387
Labels
kind/question Category issues related to questions or problems

Comments

@ZShUn
Copy link

ZShUn commented Apr 18, 2024

需求:
通过Agent下沉通用限流逻辑

问题复现步骤:
Agent中依赖sentinel,通过埋点实现通用资源限流,然后在微服务中继续依赖sentinel。当sentinel spi初始化时会加载出来两个文件路径,然后重复加载出现异常

agent中依赖:
image
image

微服务中依赖:
image

@ZShUn
Copy link
Author

ZShUn commented Apr 18, 2024

940b06a6577435661df2e012d1fd1431

@ZShUn
Copy link
Author

ZShUn commented Apr 18, 2024

应用服务 Sentinel文件路径:
jar:file:/D:/xxx/maven/localRepository/com/alibaba/csp/sentinel-core/1.8.7/sentinel-core-1.8.7.jar!/META-INF/services/com.alibaba.csp.sentinel.slotchain.ProcessorSlot

Agent Sentinel 文件路径:
jar:file:/D:/project/xxx/agent/xxx-agent-0.0.1-SNAPSHOT.jar!/META-INF/services/com.alibaba.csp.sentinel.slotchain.ProcessorSlot

@LearningGp
Copy link
Collaborator

初步看起来原因是 JVM 中存在两份 Sentinel ,我理解这是不太合理的,即便 spi 这边不抛出异常,后续功能也是未定义的可能存在很多问题,最好通过 Agent 来剔除应用服务中的依赖,使得JVM中只有一份 Sentinel。

@LearningGp LearningGp added the kind/question Category issues related to questions or problems label Apr 18, 2024
@ZShUn
Copy link
Author

ZShUn commented Apr 18, 2024

SpiLoader中的异常实际是防止重复初始化,我们只需要保证初始化一次就好。因为如果不在agent中依赖sentinel,那么就无法把通用限流功能下沉。比如根据CPU或内存进行限流

@ZShUn
Copy link
Author

ZShUn commented Apr 18, 2024

可以看看这个pr,#3377

@ZShUn
Copy link
Author

ZShUn commented Apr 19, 2024

#3185
#3181
此类问题有已经有多个issues了,因为在实际开发中,无法避免依赖包是否会传递sentinel依赖过来,如果按照你的说法,那么用户每次都需要把传递依赖排除

@ZShUn
Copy link
Author

ZShUn commented Apr 21, 2024

#1379
#1383
这是1.8.1做的spi增强pr和为什么要做spi增强,并不存在你所说的后续有什么问题。这次修复的pr也是同一个人

@cdfive
Copy link
Collaborator

cdfive commented Apr 26, 2024

@LearningGp @ZShUn
#1383 是2020年开发的,当时参考了Dubbo和Sofa框架的SPI,对Sentinel原有使用JDK的SPI做了增强,为了后续方便扩展。

为避免多个SPI实现冲突问题,当时考虑的是classLoader.getResource如果遇到重复的情况,检查出后抛出异常提示,这样让开发者早点发现问题,去解决冲突。

在工作中的项目也有使用到,因为项目里Sentinel依赖是唯一的,也没有使用agent,线上一直稳定运行没有遇到这个问题。

抱歉确实没想到有项目里存在多个Sentinel包的情况,钉钉群里看到@ZShUn反馈这个场景,在#3377 对此进行修复。

@ZShUn SpiLoader中的异常实际是防止重复初始化,我们只需要保证初始化一次就好

赞同该思路;
想到的处理方法:检查到SPI实现类重复时,程序不抛异常,在日志中记录该情况便于排查,已解析过的扩展类跳过处理,让程序正常运行。相对以前抛异常的方式,这样可能灵活友好一些,不响应SPI的功能和程序的运行。

注:以前是通过classMap里的key(Spi注解里的value别名或者实现类名称)来验证重复的,这是导致本场景出现问题的原因,因为本场景是jar包重复,即多个文件里实现类名重复,而不是别名,应该通过类或类名来验证。因此修改后的代码在该验证的前面通过classMap的value(实现类)来判断是否重复,如果重复则跳过继续处理;如果是不同实现类,Spi里的value重复,还是抛异常提示,因为这种是实现类的别名有冲突,提醒开发者去及时处理,它跟jar包重复这个场景不同;这种场景如果也跳过处理,则有优先级的问题,不确定是具体哪个SPI实现类优先。

maven构建项目可使用<dependencyManagement>统一管理依赖包版本,让项目里Sentinel相关jar唯一且版本号一致;但同时使用agent的情况,导致项目里的Sentinel包重复,进一步SPI文件重复。需修改SpiLoader的处理方式,提高容错性、兼容性。

@cdfive
Copy link
Collaborator

cdfive commented Apr 26, 2024

#3185 #3181 此类问题有已经有多个issues了,因为在实际开发中,无法避免依赖包是否会传递sentinel依赖过来,如果按照你的说法,那么用户每次都需要把传递依赖排除

确实,可能有依赖传递问题,实际开发中很常见。应修改SpiLoader处理方式,让用户体验更好。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Category issues related to questions or problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants