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

feature: add Apache Dubbo3 Support #4779

Merged
merged 9 commits into from Aug 29, 2022
Merged

Conversation

AlbumenJ
Copy link
Member

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@funky-eyes
Copy link
Contributor

个人暂时不倾向增加module的方式,我们倾向后期弄个seata-integration仓库,不再将integration放在seata仓库中随seata版本发版,而是拆开更灵活的根据微服务组件如果dubbo.grpc等迭代而迭代,这样用户仅需引入对应的seata-dubbo依赖即可
另外问一嘴,RpcContext.getContext()以及 RpcContext.getServerContext()在dubbo3上已经不需要手动清除了是吗?

@AlbumenJ
Copy link
Member Author

@a364176773

个人暂时不倾向增加module的方式,我们倾向后期弄个seata-integration仓库,不再将integration放在seata仓库中随seata版本发版,而是拆开更灵活的根据微服务组件如果dubbo.grpc等迭代而迭代,这样用户仅需引入对应的seata-dubbo依赖即可

这个我暂时没看到有对应建好的仓库,所以提交到这里了,如果有更高的存放地方请和我说

另外问一嘴,RpcContext.getContext()以及 RpcContext.getServerContext()在dubbo3上已经不需要手动清除了是吗?

是的,但是由于 seata 是基于 filter 来做的扩展点,不建议还使用 RpcContext,否则很容易和 Dubbo 处理 RpcContext 的 filter 出现顺序影响

有没有什么别的方式可以知道当前dubbo的version,加个if,让ApacheDubboTransactionPropagationFilter不去调用会出现NoSuchMethodError的这俩方法是不是也可以?

这个 PR 里面不止修改了 RpcContext 的使用,还修改了 Filter 实现的扩展点,客户端侧从 Filter 改成了选址前的 ClusterFilter。在 RPC 重试等场景下不会重入。

@funky-eyes
Copy link
Contributor

@a364176773

个人暂时不倾向增加module的方式,我们倾向后期弄个seata-integration仓库,不再将integration放在seata仓库中随seata版本发版,而是拆开更灵活的根据微服务组件如果dubbo.grpc等迭代而迭代,这样用户仅需引入对应的seata-dubbo依赖即可

这个我暂时没看到有对应建好的仓库,所以提交到这里了,如果有更高的存放地方请和我说

另外问一嘴,RpcContext.getContext()以及 RpcContext.getServerContext()在dubbo3上已经不需要手动清除了是吗?

是的,但是由于 seata 是基于 filter 来做的扩展点,不建议还使用 RpcContext,否则很容易和 Dubbo 处理 RpcContext 的 filter 出现顺序影响

有没有什么别的方式可以知道当前dubbo的version,加个if,让ApacheDubboTransactionPropagationFilter不去调用会出现NoSuchMethodError的这俩方法是不是也可以?

这个 PR 里面不止修改了 RpcContext 的使用,还修改了 Filter 实现的扩展点,客户端侧从 Filter 改成了选址前的 ClusterFilter。在 RPC 重试等场景下不会重入。

仓库问题之后会建,用户排除1.5.1的seata-dubbo模块自行引入1.4.2的也是可以用的
所以我更在意的是,是否有一个通用方案兼容2.7.x和3.x的方案,ClusterFilter 在2.7.x是不是无法使用,我好像在2.7.9的版本上没有看到这个接口,即便rpc场景重入其实对结果没什么影响,只不过多做了一些重复处理,因为我们会在finally里removeAttachment进行处理

@funky-eyes
Copy link
Contributor

@a364176773

个人暂时不倾向增加module的方式,我们倾向后期弄个seata-integration仓库,不再将integration放在seata仓库中随seata版本发版,而是拆开更灵活的根据微服务组件如果dubbo.grpc等迭代而迭代,这样用户仅需引入对应的seata-dubbo依赖即可

这个我暂时没看到有对应建好的仓库,所以提交到这里了,如果有更高的存放地方请和我说

另外问一嘴,RpcContext.getContext()以及 RpcContext.getServerContext()在dubbo3上已经不需要手动清除了是吗?

是的,但是由于 seata 是基于 filter 来做的扩展点,不建议还使用 RpcContext,否则很容易和 Dubbo 处理 RpcContext 的 filter 出现顺序影响

有没有什么别的方式可以知道当前dubbo的version,加个if,让ApacheDubboTransactionPropagationFilter不去调用会出现NoSuchMethodError的这俩方法是不是也可以?

这个 PR 里面不止修改了 RpcContext 的使用,还修改了 Filter 实现的扩展点,客户端侧从 Filter 改成了选址前的 ClusterFilter。在 RPC 重试等场景下不会重入。

至于说的不建议使用RpcContext,据我所知某些dubbo的版本RpcContext不一定会清除相关的隐式传参的内容,所以我们这边是统一加上,如果说会跟dubbo自身的处理起冲突,是否可以调整order?

@AlbumenJ
Copy link
Member Author

至于说的不建议使用RpcContext,据我所知某些dubbo的版本RpcContext不一定会清除相关的隐式传参的内容,所以我们这边是统一加上,如果说会跟dubbo自身的处理起冲突,是否可以调整order?

这里直接不要使用了不就好了,另外 Dubbo 的 RpcContext 处理优先级是最高的,其他 Filter 很难提高优先级,而且不好说未来 dubbo 这块又需要调整,然后就裂开了,从 invocation 传就没问题了

@funky-eyes
Copy link
Contributor

以前就想着不使用,有一次跟你线下沟通还记得吗,相关issue #3844
因为难免rpc框架有bug,我们手动清空能保证我们自身不被其处理逻辑有的bug影响,至于说的dubbo的filter优先级是最高的,那我个人感觉这里我们只清理了我们自己的key,应该不会对其造成什么大影响吧?

@funky-eyes
Copy link
Contributor

我个人想法是这个pr最好能兼容2.7.x和3.x,而不是另起module,如果是的,等我们弄个新的仓库,或者把这块从seata-allmodule里移除掉,将各自对应的版本重新改版本号,比如现在的seata中的alibabadubbo模块直接发个2.5.x的版本后删掉,再将apache-dubbo版本保留,发一个2.7.x的版本,直接迭代到3.x,发一个3.x的版本,未来直接跟dubbo的版本号走,再在官网标注使用不同微服务框架需要使用的xid传递的module的版本号,总的来说2个事
1.拆模块
2.跟apache-dubbo版本走
话说如果拆掉,可以把这个仓库直接放到dubbo下吗,就跟spring-cloud-alibaba那边一样,我们参与一起维护?这样就最好了版本号都是跟着dubbo自身走,用户用的也比较放心
如果放到seata侧要再等一下,等拆出来后,河清兄你这边再交个代码,我们整进来发个3.x的seata-dubbo

@AlbumenJ
Copy link
Member Author

@a364176773

个人暂时不倾向增加module的方式,我们倾向后期弄个seata-integration仓库,不再将integration放在seata仓库中随seata版本发版,而是拆开更灵活的根据微服务组件如果dubbo.grpc等迭代而迭代,这样用户仅需引入对应的seata-dubbo依赖即可

这个我暂时没看到有对应建好的仓库,所以提交到这里了,如果有更高的存放地方请和我说

另外问一嘴,RpcContext.getContext()以及 RpcContext.getServerContext()在dubbo3上已经不需要手动清除了是吗?

是的,但是由于 seata 是基于 filter 来做的扩展点,不建议还使用 RpcContext,否则很容易和 Dubbo 处理 RpcContext 的 filter 出现顺序影响

有没有什么别的方式可以知道当前dubbo的version,加个if,让ApacheDubboTransactionPropagationFilter不去调用会出现NoSuchMethodError的这俩方法是不是也可以?

这个 PR 里面不止修改了 RpcContext 的使用,还修改了 Filter 实现的扩展点,客户端侧从 Filter 改成了选址前的 ClusterFilter。在 RPC 重试等场景下不会重入。

仓库问题之后会建,用户排除1.5.1的seata-dubbo模块自行引入1.4.2的也是可以用的 所以我更在意的是,是否有一个通用方案兼容2.7.x和3.x的方案,ClusterFilter 在2.7.x是不是无法使用,我好像在2.7.9的版本上没有看到这个接口,即便rpc场景重入其实对结果没什么影响,只不过多做了一些重复处理,因为我们会在finally里removeAttachment进行处理

以前就想着不使用,有一次跟你线下沟通还记得吗,相关issue #3844

上次那个我看了下记录,背景是在 Dubbo2 里面这样写入了就需要清理,Dubbo3 做了隔离,这个逻辑不需要删除了。另外我看了下 RpcContext 在这边的用法,用 invocation 直接写入是完全可以达到目标的(这块和 Dubbo2是兼容的)

所以我更在意的是,是否有一个通用方案兼容2.7.x和3.x的方案,ClusterFilter 在2.7.x是不是无法使用,我好像在2.7.9的版本上没有看到这个接口,即便rpc场景重入其实对结果没什么影响,只不过多做了一些重复处理,因为我们会在finally里removeAttachment进行处理

这个也可以改成和 Dubbo 2 一起用的写法,只是把 ClusterFilter 改成 Filter 其他 API 都是 Dubbo2 里面兼容的。但是改成 ClusterFilter 后长期会存在内存占用上的问题(Dubbo 内部的机制导致的,所以在客户端侧 Filter 能不用就不用,都改用 ClusterFilter)

可以这样,先把现在 Dubbo2 的那个实现改成 3 也兼容的写法,然后 Dubbo3 的再一个独立模块,这样有个迁移的路径

@AlbumenJ
Copy link
Member Author

我个人想法是这个pr最好能兼容2.7.x和3.x,而不是另起module,如果是的,等我们弄个新的仓库,或者把这块从seata-allmodule里移除掉,将各自对应的版本重新改版本号,比如现在的seata中的alibabadubbo模块直接发个2.5.x的版本后删掉,再将apache-dubbo版本保留,发一个2.7.x的版本,直接迭代到3.x,发一个3.x的版本,未来直接跟dubbo的版本号走,再在官网标注使用不同微服务框架需要使用的xid传递的module的版本号,总的来说2个事 1.拆模块 2.跟apache-dubbo版本走 话说如果拆掉,可以把这个仓库直接放到dubbo下吗,就跟spring-cloud-alibaba那边一样,我们参与一起维护?这样就最好了版本号都是跟着dubbo自身走,用户用的也比较放心 如果放到seata侧要再等一下,等拆出来后,河清兄你这边再交个代码,我们整进来发个3.x的seata-dubbo

版本迁移的我前面回复了,出个兼容 2.7.x 的版本是可以的,但是 Dubbo 3 下面会有性能下降,可以一步一步走

放到 dubbo 仓库也是可以的,可以放到 apache/dubbo-spi-extensions 下面,但是后续 seata 发版本也需要测试和 Dubbo 侧的兼容性,Dubbo 这边合入 dubbo-spi-extensions 后每个小迭代都会测试兼容性

@funky-eyes
Copy link
Contributor

我个人想法是这个pr最好能兼容2.7.x和3.x,而不是另起module,如果是的,等我们弄个新的仓库,或者把这块从seata-allmodule里移除掉,将各自对应的版本重新改版本号,比如现在的seata中的alibabadubbo模块直接发个2.5.x的版本后删掉,再将apache-dubbo版本保留,发一个2.7.x的版本,直接迭代到3.x,发一个3.x的版本,未来直接跟dubbo的版本号走,再在官网标注使用不同微服务框架需要使用的xid传递的module的版本号,总的来说2个事 1.拆模块 2.跟apache-dubbo版本走 话说如果拆掉,可以把这个仓库直接放到dubbo下吗,就跟spring-cloud-alibaba那边一样,我们参与一起维护?这样就最好了版本号都是跟着dubbo自身走,用户用的也比较放心 如果放到seata侧要再等一下,等拆出来后,河清兄你这边再交个代码,我们整进来发个3.x的seata-dubbo

版本迁移的我前面回复了,出个兼容 2.7.x 的版本是可以的,但是 Dubbo 3 下面会有性能下降,可以一步一步走

放到 dubbo 仓库也是可以的,可以放到 apache/dubbo-spi-extensions 下面,但是后续 seata 发版本也需要测试和 Dubbo 侧的兼容性,Dubbo 这边合入 dubbo-spi-extensions 后每个小迭代都会测试兼容性

当前版本1.5.2及之前,seata携带的都是自行维护的seata-dubbo的xid传递模块,后续版本放到你们那边的一个spi扩展仓库,我们共同维护测试,后续seata直接引用dubbo侧的这个新的模块,版本号直接与dubbo版本号对齐,怎么样?这样分支和版本管理就比较清晰了,可以2.7.x的一个分支,3.x的一个分支和dubbo主仓库一样,各自维护不同版本并行进行

dependencies/pom.xml Outdated Show resolved Hide resolved
@funky-eyes funky-eyes added this to the 1.6.0 milestone Aug 11, 2022
@funky-eyes funky-eyes changed the title Add Apache Dubbo3 Support featrue: Add Apache Dubbo3 Support Aug 11, 2022
@funky-eyes funky-eyes changed the title featrue: Add Apache Dubbo3 Support feature: add Apache Dubbo3 Support Aug 11, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #4779 (efbdad6) into develop (06c2a50) will decrease coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4779      +/-   ##
=============================================
- Coverage      49.23%   49.03%   -0.20%     
+ Complexity      4117     4088      -29     
=============================================
  Files            737      736       -1     
  Lines          25827    25784      -43     
  Branches        3193     3180      -13     
=============================================
- Hits           12715    12644      -71     
- Misses         11764    11798      +34     
+ Partials        1348     1342       -6     
Impacted Files Coverage Δ
...va/io/seata/server/console/vo/GlobalSessionVO.java 22.05% <0.00%> (-33.83%) ⬇️
...java/io/seata/server/storage/SessionConverter.java 80.00% <0.00%> (-9.10%) ⬇️
...rage/redis/store/RedisTransactionStoreManager.java 63.17% <0.00%> (-3.40%) ⬇️

@funky-eyes funky-eyes added first-time contributor first-time contributor module/integration integration module labels Aug 27, 2022
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes funky-eyes merged commit 8ec5ccd into apache:develop Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/integration integration module multilingual
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants