sofastack / sofa-bolt

SOFABolt is a lightweight, easy to use and high performance remoting framework based on Netty.
https://www.sofastack.tech/projects/sofa-bolt/
Apache License 2.0
2.44k stars 860 forks source link

improve: write and flush methods are provided #346

Closed funky-eyes closed 7 months ago

funky-eyes commented 7 months ago

https://github.com/funky-eyes/redispike-proxy 自己写的一个解析redis协议写入aerospike的代理程序,依赖sofa-bolt。支持redispippline时,需要批量响应,接收N个消息后,需要最后一个消息写完再flush,而sofa-bolt目前无法直接调用到write和单独的flush故此pr开放相关功能,并且利用这个功能,理论上也可以优化吞吐量,比如dubbo3.2的10倍性能提升就是通过先投入内存队列,然后异步从队列pop,将队列中同一时刻的请求全部先write,然后执行flush。 直接使用ctx#getChannelContext 粒度有点大,也不太优雅

Summary by CodeRabbit

coderabbitai[bot] commented 7 months ago

Walkthrough

The recent modifications enhance the functionality of the RemotingContext class by introducing new methods that encapsulate the write and flush operations. This update improves the code's readability and structure, making it more maintainable and clear.

Changes

File Path Change Summary
src/.../RemotingContext.java Added methods for write and flush operations, improved code structure.

🐇✨ In the land of code, where the bits roam free, A rabbit hopped in, with changes to see. "Write and flush," it said with glee, "Let’s wrap them up, more clearly to be!" Now the code flows smooth, like a gentle sea. 🌊 🐇✨


Recent Review Details **Configuration used: CodeRabbit UI**
Commits Files that changed from the base of the PR and between 138f9e4edf59104ce6d5fb4df1013631e871bbf9 and 15119bc461eb3f2fc2de8aae8f22ccb19b7fe080.
Files selected for processing (1) * src/main/java/com/alipay/remoting/RemotingContext.java (1 hunks)
Additional comments not posted (3)
src/main/java/com/alipay/remoting/RemotingContext.java (3)
`108-109`: LGTM! The `writeAndFlush` method is well-implemented and documented. --- `116-122`: LGTM! The `write` method is correctly implemented and documented, aligning with the PR's objectives. --- `126-130`: Consider enhancing the documentation for the `flush` method to explain what flushing the channel does, which could be helpful for maintainability and clarity.
--- Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.
OrezzerO commented 7 months ago

我觉得使用 ctx#getChannelContext 挺好的,这代表你已经使用了 Netty 的特性了, 超出了bolt 的范畴. 减少了误用RemotingContext 的可能性

funky-eyes commented 7 months ago

我觉得使用 ctx#getChannelContext 挺好的,这代表你已经使用了 Netty 的特性了, 超出了bolt 的范畴. 减少了误用RemotingContext 的可能性

如果是这样的设计,为什么还要对外单独开放一个writeAndFlush呢?我觉得如果要做的比较合理,比如屏蔽一些容易误操作的功能,应该把可用功能开放,风险高的功能封闭,或者框架层调优,无需用户去感知。

OrezzerO commented 7 months ago

在我看来, writeAndFlush 并不是一个高风险的方法; 但是 write 和 flush 的组合就是一个比较容易出错的组合. Bolt 屏蔽了很多 Netty 的细节, 所以我们也不能默认 Bolt 的使用者对于 netty 很熟悉, 分别引入 write 和 flush 方法的必要性不是很大. 而且现在我们有替代的方法 ctx.getChannelContext().write()ctx.getChannelContext().flush() 我觉得这已经足够了.

chuailiwu commented 7 months ago

看了下,感觉每个人理解不一样,我是感觉RemotingContext这里不应该提供getChannelContext方法,而是把必要的方法、属性按需开放出来更合理,我看代码里这个方法基本都是为了获取远程地址String remotingAddress = RemotingUtil.parseRemoteAddress(ctx.getChannelContext() .channel());