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

发现的一些小问题 #9

Open
smile2coder opened this issue Aug 9, 2020 · 5 comments
Open

发现的一些小问题 #9

smile2coder opened this issue Aug 9, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@smile2coder
Copy link
Contributor

我个人很喜欢这个项目,因为它能帮助我更好的理解rpc的细节。前段时间我看了李林峰的 《Netty权威指南》,也自己实现过简单的rpc,但是却不如你的全面和细节。在阅读源码的过程中,我发现了一下小问题,也可能是我没有get到你的思路,请指教

  1. github.javaguide.factory.SingletonFactory#getInstance,这个单例只是实现了一次校验,所以它恐怕不能正确的运行。
  2. github.javaguide.serialize.kyro.KryoSerializer#kryoThreadLocal,因为Kryo是非线程安全的,所以用了ThreadLocal为每个线程创建一个实例。由于netty的序列化操作是在线程池中运行,所以没必要每次序列化完成后都做remove操作。
  3. 我看项目中实现了以注解的方式发布服务,主要原理是用了SpringBeanPostProcessor对实例做了后置处理,但是SpringBeanPostProcessor能注册到容器原因是,你的示例项目example-server中NettyServerMain类中含有@componentscan注解,扫描的包名与rpc-framework-simple项目相同而已,这显然是不合理的。
  4. 许多资源的创建没有加锁,比如github.javaguide.registry.zk.util.CuratorUtils#getZkClient。当然,这些可以靠spring完成。

个人的一些理解,如有不足,还望指教

@Snailclimb
Copy link
Owner

我个人很喜欢这个项目,因为它能帮助我更好的理解rpc的细节。前段时间我看了李林峰的 《Netty权威指南》,也自己实现过简单的rpc,但是却不如你的全面和细节。在阅读源码的过程中,我发现了一下小问题,也可能是我没有get到你的思路,请指教

  1. github.javaguide.factory.SingletonFactory#getInstance,这个单例只是实现了一次校验,所以它恐怕不能正确的运行。
  2. github.javaguide.serialize.kyro.KryoSerializer#kryoThreadLocal,因为Kryo是非线程安全的,所以用了ThreadLocal为每个线程创建一个实例。由于netty的序列化操作是在线程池中运行,所以没必要每次序列化完成后都做remove操作。
  3. 我看项目中实现了以注解的方式发布服务,主要原理是用了SpringBeanPostProcessor对实例做了后置处理,但是SpringBeanPostProcessor能注册到容器原因是,你的示例项目example-server中NettyServerMain类中含有@componentscan注解,扫描的包名与rpc-framework-simple项目相同而已,这显然是不合理的。
  4. 许多资源的创建没有加锁,比如github.javaguide.registry.zk.util.CuratorUtils#getZkClient。当然,这些可以靠spring完成。

个人的一些理解,如有不足,还望指教

很赞啊!老哥!有些点的话确实没处理好。有时间的话还会继续优化以及修复现有的问题和你说的问题。🤟

@Snailclimb
Copy link
Owner

我个人很喜欢这个项目,因为它能帮助我更好的理解rpc的细节。前段时间我看了李林峰的 《Netty权威指南》,也自己实现过简单的rpc,但是却不如你的全面和细节。在阅读源码的过程中,我发现了一下小问题,也可能是我没有get到你的思路,请指教

  1. github.javaguide.factory.SingletonFactory#getInstance,这个单例只是实现了一次校验,所以它恐怕不能正确的运行。
  2. github.javaguide.serialize.kyro.KryoSerializer#kryoThreadLocal,因为Kryo是非线程安全的,所以用了ThreadLocal为每个线程创建一个实例。由于netty的序列化操作是在线程池中运行,所以没必要每次序列化完成后都做remove操作。
  3. 我看项目中实现了以注解的方式发布服务,主要原理是用了SpringBeanPostProcessor对实例做了后置处理,但是SpringBeanPostProcessor能注册到容器原因是,你的示例项目example-server中NettyServerMain类中含有@componentscan注解,扫描的包名与rpc-framework-simple项目相同而已,这显然是不合理的。
  4. 许多资源的创建没有加锁,比如github.javaguide.registry.zk.util.CuratorUtils#getZkClient。当然,这些可以靠spring完成。

个人的一些理解,如有不足,还望指教

很赞啊!老哥!有些点的话确实没处理好。有时间的话还会继续优化以及修复现有的问题和你说的问题。🤟

你要是有时间的话也可以小步pr给我。嘿嘿。

@Snailclimb Snailclimb added the enhancement New feature or request label Aug 10, 2020
@Snailclimb
Copy link
Owner

第三个自定义注解的问题已经解决,commit在这里:292c64d
主要是新增加了可以扫描自定义注解的注解: RpcScan
image
image

@smile2coder
Copy link
Contributor Author

哈哈,相互学习🤣 ,不得不说,你解决问题的速度是真的快啊!如果再发现问题会提pr的,奥利给

@zzzczh
Copy link

zzzczh commented May 18, 2021

您好 想请教一个问题。我对两位说的 因为Kryo是非线程安全的,所以用了ThreadLocal为每个线程创建一个实例这句话有所疑惑。我认为换种方式直接在serialize方法内Kryo kryo = new Kryo();似乎也能起到同样的效果,每个线程都能且只能修改到一个kryo实例,即栈隔离的方式。这样是否就能避免使用ThreadLocal?尽管性能上并没有更佳,但也没有线程安全问题。

Xiao0731 added a commit to Xiao0731/guide-rpc-framework that referenced this issue Jul 20, 2024
Snailclimb added a commit that referenced this issue Jul 22, 2024
Fix issue #9:解决了issue里的前两个问题,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants