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

使用tonic实现RPC #375

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

Jackhr-arch
Copy link

@Jackhr-arch Jackhr-arch commented Mar 5, 2025

Close #322

Todo

  • 更新track_id(session_id)生成方式(使用uuid_v7
  • 创建并维护日志池(与状态池合并)
  • 创建并维护状态池
  • 将MaaCore日志目录设置合并到load_core中(可以指定绝对路径或相对路径)
  • 提供RESTful api
  • 合并部分cli功能
  • 允许远程关闭并“优雅地退出”
  • 捕获kill以便在服务端退出前卸载MaaCore

需要讨论的部分

提供RESTful api

目前有三种方案

  • 使用axum接受json,转换为protobuf后转发给tonic
    好处是可以选择是否开启http端口及其服务,坏处是需要手动编写路由及转发(可以考虑展开宏?),还有一定的性能损失(无伤大雅)

  • 修改tonic-build,使tonic可以自动根据传入的content-type切换codec
    好处是无需额外代码,坏处是需要使用自定义的tonic-build库(crate.io-patch)(至少在我想办法把它合并到tonic官方库前),以及只能监听一个端口,要么本地socket,要么http端口

  • 构造一个tower中间件,将传入的content-type为json的请求从json转换为protobuf
    不用重新路由,不必修改tonic-build,但只能监听一个端口且有性能损失

合并部分cli功能

这些功能将以features的形式实现

  • 自更新,更新MaaCore,相关资源(作为一个lib,无需实现)

Notice

这是在 #374 的基础上编写的,所以请忽略其对maa-cli的修改

@wangl-cc
Copy link
Member

wangl-cc commented Mar 5, 2025

提供RESTful api

目前有三种方案

* 使用`axum`接受json,转换为protobuf后转发给tonic
  好处是可以选择是否开启http端口及其服务,坏处是需要手动编写路由及转发(可以考虑展开宏?),还有一定的性能损失(无伤大雅)

* 修改`tonic-build`,使tonic可以自动根据传入的`content-type`切换codec
  好处是无需额外代码,坏处是需要使用自定义的tonic-build库(crate.io-patch)(至少在我想办法把它合并到tonic官方库前),以及只能监听一个端口,要么本地socket,要么http端口

* 构造一个tower中间件,将传入的`content-type`为json的请求从json转换为protobuf
  不用重新路由,不必修改tonic-build,但只能监听一个端口且有性能损失

Web 不一定需要 RESTful API,web 可以走 grpc-web,然后 tonic 可以通过 tonic-web 来提供对 grpc-web 的支持。这样的方案比较成熟。

@Jackhr-arch
Copy link
Author

Web 不一定需要 RESTful API,web 可以走 grpc-web,然后 tonic 可以通过 tonic-web 来提供对 grpc-web 的支持。这样的方案比较成熟。

那好,我会把这个的优先级延后。

@Jackhr-arch
Copy link
Author

关于状态池

目前的处理是subtask对应的变更直接保存,以便客户端“复原”执行过程

事实上我在考虑合并日志池和状态池,因为他们的功能相近(至少目前如此)

@Jackhr-arch
Copy link
Author

Hi, @BoredTape, 我正在使用tonic为maa-cli实现RPC服务器,你可以给出一些建议吗

@Jackhr-arch Jackhr-arch requested a review from wangl-cc March 6, 2025 15:31
@Jackhr-arch
Copy link
Author

关于CI

由于tonic-build依赖protoc,因此在手动安装protoc前编译不会通过

各个平台的protoc安装方式也已经在构造日志中给出(除了windows那个奇葩)

@wangl-cc
Copy link
Member

wangl-cc commented Mar 6, 2025

我看了一下,有好多代码是用来转换 Protobuf 生成的类型和 maa-types 类型的。我在 #379 里给 TouchMode 和 TaskType 手动实现了 prost。你看看能不能行?

@wangl-cc
Copy link
Member

wangl-cc commented Mar 6, 2025

我有一问题就是为什么要 server 和 client 两个 binary。我觉得这个 crate 应该就是一个 lib,然后提供一个函数来实现启动 server。客户端的代码不应该包含在这个库里面。如果这两个是用来测试的我觉得可以单独写一个集成测试。

我觉得这个 crate 不要依赖于 maa-dirs。Core,资源,以及 MaaCore 日志的位置,应该作为启动 server 函数的参数提供。不过在测试里面可以用 maa-dirs 来找到默认位置。

@Jackhr-arch
Copy link
Author

我有一问题就是为什么要 server 和 client 两个 binary。我觉得这个 crate 应该就是一个 lib,然后提供一个函数来实现启动 server。客户端的代码不应该包含在这个库里面。如果这两个是用来测试的我觉得可以单独写一个集成测试。

是的,目前这个库属于测试例。在所有功能完成后我会修改其结构

我觉得这个 crate 不要依赖于 maa-dirs。Core,资源,以及 MaaCore 日志的位置,应该作为启动 server 函数的参数提供。不过在测试里面可以用 maa-dirs 来找到默认位置。

目前仅有lib.rs/mod util依赖maa-dirs,也就是加载maacore与资源的功能。我会处理这部分

@BoredTape
Copy link

Hi, @BoredTape, 我正在使用tonic为maa-cli实现RPC服务器,你可以给出一些建议吗

目前我在为生活做斗争,前段时间把电脑卖了,等我有钱买电脑我会继续做我那边的工作,目前来说,tonic实现RPC服务器的话,足够我这边的使用了

@Jackhr-arch
Copy link
Author

Jackhr-arch commented Mar 7, 2025

关于内存

  • 现在使用unload_core作为服务端关闭入口。但没有捕获Ctrl-C,所以kill服务端依旧会导致MaaCore无法卸载导致内存泄漏(使用tokio::singal捕获事件,确保unload_core运行)
  • 已知存在一个内存泄漏点crates/maa-server/src/server_impl/task.rs: mod wrapper: Assistant::new。为了确保指向SessionID的指针有效,这里执行了一次手动内存泄漏,而且目前没有什么好的方式回收。(现在在callback中与Session一起回收)

@wangl-cc
Copy link
Member

wangl-cc commented Mar 7, 2025

有好多无关的格式变化,可能需要你重新格式化一下。格式化配置用了很多不稳定特性,需要 rustfmt nightly 来格式化。

@Jackhr-arch Jackhr-arch marked this pull request as ready for review March 8, 2025 06:43
VideoRecognition = 15;
}

enum TaskStateType {
Copy link
Member

Choose a reason for hiding this comment

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

这个 enum 的值不对。

Copy link
Author

Choose a reason for hiding this comment

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

其实改不改无所谓。

  • 在服务端,从AsstMsgId转换到TaskStateType,实际上是根据maa-types/lib.rs里定义的值,那里的数值是对的
  • 对于用户端,他们无须关心具体值。

不过,为了避免后面混淆了,我还是改一改吧

@Jackhr-arch Jackhr-arch requested a review from wangl-cc March 9, 2025 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

maa-cli 前后端分离
3 participants