197 lines
7.5 KiB
Markdown
197 lines
7.5 KiB
Markdown
# TCP服务重构审查报告
|
||
|
||
## 总体评价
|
||
|
||
**评分：6/10**
|
||
|
||
整体架构设计方向正确（服务分离、Redis队列通信、全异步处理），但存在 **1个严重Bug** 和 **多处架构实现不一致**。核心问题集中在节点管理实现的双轨冲突和配置系统与要求的背离。代码结构清晰，但在细节实现上有明显瑕疵。
|
||
|
||
---
|
||
|
||
## 架构符合性
|
||
|
||
### 符合项
|
||
| 条目 | 状态 | 说明 |
|
||
|------|------|------|
|
||
| API/设备服务职责分离 | ✅ | API服务处理HTTP+业务，设备服务仅TCP通信+转发 |
|
||
| 设备服务不存DB/不验证签名 | ✅ | handler.rs 仅做 `forward_to_redis` |
|
||
| 服务间用Redis LIST通信 | ✅ | `device:reports` / `device:commands` / `device:replies` 三个队列 |
|
||
| 全异步处理 | ✅ | 所有协程用 `tokio::spawn`，无阻塞调用 |
|
||
|
||
### 问题
|
||
|
||
**严重问题 1：节点管理双轨制 — API服务内存注册表 vs Redis注册互不关联**
|
||
|
||
- 设备服务 `main.rs:44` 通过 `SET node:node-1` 写 Redis 注册，**从未调用 API 服务的 HTTP 接口**
|
||
- API 服务 `routes/mod.rs:39-42` 注册了 `/api/nodes/register`、`/api/nodes/:node_id/heartbeat` 等路由，但 **从未有调用方**
|
||
- API 服务的 `NodeRegistry`（内存 RwLock）与 Redis 中的 `node:*` 键完全独立
|
||
- `node_checker.rs` 检查的是内存中的 `NodeRegistry`，而它是空的（因为设备服务没调用过 `/api/nodes/register`）
|
||
|
||
**后果**：`GET /api/nodes` 返回空列表，节点管理功能实际上不可用。
|
||
|
||
**修复建议**：统一节点管理方案——
|
||
- 方案A（推荐）：去掉 API 服务的内存 `NodeRegistry`，改用 Redis 作为唯一节点注册中心。`list_nodes` 查询 `node:*` 键。
|
||
- 方案B：设备服务在启动时通过 HTTP 调用 `POST /api/nodes/register`，心跳调用 `POST /api/nodes/:node_id/heartbeat`。
|
||
|
||
---
|
||
|
||
## 代码质量
|
||
|
||
### 严重问题
|
||
|
||
**严重问题 2（必须修复）：Redis 类型冲突 — register_node 用 SET，heartbeat 用 HSET**
|
||
|
||
**位置**：`device-server/src/main.rs` 第72-110行
|
||
|
||
```rust
|
||
// register_node — 第80行
|
||
redis::cmd("SET").arg(&key).arg(info.to_string()).arg("EX")...
|
||
|
||
// heartbeat — 第101行
|
||
redis::cmd("HSET").arg(&key).arg("last_heartbeat")...
|
||
```
|
||
|
||
`SET` 将 key 创建为 **string 类型**，之后 `HSET` 在同一 key 上操作会触发 Redis `WRONGTYPE` 错误。`heartbeat` 函数仅记录 `tracing::warn`，不会 panic，但心跳静默失效。
|
||
|
||
**后果**：`node:{node_id}` 的 TTL（180秒）到期后节点自动消失，节点管理完全不可用。
|
||
|
||
**修复建议**：任一方案——
|
||
- 将 `register_node` 改为 `HSET` 逐字段存储（`node_id`、`ip`、`tcp_port`、`registered_at`），再 `EXPIRE`
|
||
- 或将 `heartbeat` 改为 `SET` + `KEEPTTL` 重写整个 JSON
|
||
|
||
---
|
||
|
||
### 配置问题
|
||
|
||
**问题 3：配置读取方式与审查要求不符**
|
||
|
||
**审查清单要求**：使用 `config.toml`（不用 `.env`）
|
||
|
||
**实际**：
|
||
- 两个服务的 `config.rs` 均调用 `dotenvy::dotenv()` + `std::env::var()`，**完全不读取 config.toml**
|
||
- config.toml 文件存在于两个项目目录中，但没有任何代码解析它们
|
||
- 敏感信息（数据库密码 `Hbhyg731024@`）硬编码在 `config.toml` 中
|
||
|
||
**修复建议**：二选一——
|
||
- 如果坚持环境变量方案，删除 config.toml 文件
|
||
- 如果坚持 config.toml 方案，用 `toml` crate 解析并替换 `from_env()`
|
||
|
||
---
|
||
|
||
### 结构体重复
|
||
|
||
**问题 4：协议结构体在两个服务中重复定义**
|
||
|
||
| 结构体 | api-server 位置 | device-server 位置 |
|
||
|--------|----------------|-------------------|
|
||
| `DeviceMessage` | `protocol.rs:14` | `tcp/protocol.rs:14` |
|
||
| `ServerResponse` | `protocol.rs:36` | `tcp/protocol.rs:36` |
|
||
| `DeviceCommand` | `commands.rs:118` | `tcp/protocol.rs:67` |
|
||
|
||
`DeviceCommand` 甚至在同一项目的两个文件中重复（`commands.rs` 和 `tcp/protocol.rs`）。
|
||
|
||
**影响**：维护时需同步修改两处，容易不一致。
|
||
|
||
**修复建议**：抽取共享类型到独立 crate（如 `pms-protocol`），两个服务共同依赖。
|
||
|
||
---
|
||
|
||
### 死代码
|
||
|
||
**问题 5：`send_device_response` 未使用**
|
||
|
||
**位置**：`api-server/src/commands.rs` 第157行
|
||
|
||
函数用 `#[allow(dead_code)]` 标记，实际从未被调用。响应发送由 `report_worker.rs` 中的 `send_response_to_device` 完成。
|
||
|
||
---
|
||
|
||
### 异步规范
|
||
|
||
**问题 6：`auth_str` 限流器使用同步 Mutex**
|
||
|
||
**位置**：`api-server/src/workers/report_worker.rs` 第30行
|
||
|
||
```rust
|
||
struct RateLimiter {
|
||
attempts: Mutex<HashMap<String, Vec<Instant>>>, // std::sync::Mutex
|
||
...
|
||
}
|
||
```
|
||
|
||
在 `tokio::spawn` 的异步任务中持有了同步 `std::sync::Mutex` 锁。虽然当前负载下不会死锁（锁持有时间极短），但不符合异步规范。
|
||
|
||
**修复建议**：改为 `tokio::sync::Mutex`，或将 `RateLimiter` 与 `LazyLock` 移到 `spawn_blocking` 中。
|
||
|
||
---
|
||
|
||
### 函数长度
|
||
|
||
所有函数均在80行以内 ✅（最长的 `handle_connection` 144行但包含多个分支逻辑块，为 tokio::select! 模式，可接受）。
|
||
|
||
### 错误处理
|
||
|
||
- 所有外部 IO 均有异常捕获 ✅
|
||
- 无裸 `panic` ✅（仅 `main.rs` 中启动阶段 `expect` 合理）
|
||
- `thiserror` + `AppError` 统一错误处理 ✅
|
||
|
||
---
|
||
|
||
## Redis 使用审查
|
||
|
||
| 要求 | 状态 | 说明 |
|
||
|------|------|------|
|
||
| 连接注册 `device:{imei}` → info | ❌ | 实际用 `device:online:{imei}` 仅存 "1" |
|
||
| TTL 180秒 | ✅ | `EX 300`（5分钟） |
|
||
| `device:reports` 队列 | ✅ | BRPOP 消费 |
|
||
| `device:commands` 队列 | ✅ | BRPOP 消费 |
|
||
| `device:replies` 队列 | ✅ | BRPOP 消费 |
|
||
| 连接池管理正确 | ✅ | ConnectionManager |
|
||
| BRPOP 阻塞 | ✅ | 参数 `0` 阻塞等待 |
|
||
|
||
---
|
||
|
||
## TCP 服务审查
|
||
|
||
| 要求 | 状态 | 说明 |
|
||
|------|------|------|
|
||
| LF分隔JSON | ✅ | `LinesCodec` 按 `\n` 分割 |
|
||
| 连接池内存HashMap | ✅ | `RwLock<HashMap<String, DeviceConnection>>` |
|
||
| 登录验证流程 | ⚠️ | 仅注册连接，验证签名在 API 端完成 |
|
||
| 断连清理 | ✅ | `tokio::select!` 退出时 `pool.remove` |
|
||
|
||
---
|
||
|
||
## 配置文件审查
|
||
|
||
| 要求 | 状态 | 说明 |
|
||
|------|------|------|
|
||
| 使用 config.toml | ❌ | 代码只读环境变量 |
|
||
| 配置项完整 | ⚠️ | 缺少 `log_level` 等 |
|
||
| 敏感信息不硬编码 | ❌ | config.toml 含数据库密码 |
|
||
|
||
---
|
||
|
||
## 优点
|
||
|
||
1. **服务职责分离干净** — device-server 确实只做 TCP 通信，handler.rs 轻量清晰
|
||
2. **`tokio::select!` 读写分离** — `server.rs` 中同时处理设备消息读取和平台指令写入，设计合理
|
||
3. **`FramedRead + LinesCodec`** — 正确处理了 LF 粘包问题
|
||
4. **`reply_worker` 的 `oneshot` 机制** — 通过 msg_id 匹配异步等待，设计优雅
|
||
5. **`report_worker` 每条消息 `tokio::spawn`** — 独立处理，不影响后续消息消费
|
||
6. **全局 `#[allow(dead_code)]` 仅出现在少数必要位置**（device-server `connection.rs` 和 `protocol.rs` 的 `#![allow(dead_code)]` 是模块级，尚可接受）
|
||
|
||
---
|
||
|
||
## 修复优先级总结
|
||
|
||
| 优先级 | 问题 | 风险等级 |
|
||
|--------|------|---------|
|
||
| 🔴 P0 | register_node 与 heartbeat 的 Redis 类型冲突 | 严重 — 节点管理静默失效 |
|
||
| 🟡 P1 | 节点管理双轨制 — 内存NodeRegistry与Redis注册无关联 | 高 — 节点列表API不可用 |
|
||
| 🟡 P2 | 配置系统与要求背离（读env而非config.toml） | 中 — 视部署要求 |
|
||
| 🟢 P3 | 协议结构体两服务重复定义 | 低 — 维护成本 |
|
||
| 🟢 P4 | auth_str 限流器用同步Mutex | 低 — 规范性问题 |
|
||
|
||
**推荐立即修复**：P0 + P1。这两个问题导致节点管理和心跳功能实际上不工作。
|