Commit 265752a19799601886d48550331ff82b94a28b52
1 parent
5084c2ef
graphlemon: sdd artifact for task 911861fe-446a-4bab-8b69-b4872b92d616
Showing
1 changed file
with
276 additions
and
5 deletions
.graphlemon/runs/911861fe-446a-4bab-8b69-b4872b92d616/sdd.md
| 1 | -The graphlemon Work Node (planner) workflow completed. It produced an SDD for the goal "修复 supervisor/deploy_stage.py 并发部署冲突缺少互斥锁的问题": | 1 | +# SDD: `supervisor/deploy_stage.py` 并发部署冲突诊断化处理 |
| 2 | 2 | ||
| 3 | -- Problem: stage_deployment() only checks the last finalized deployment, not whether one is currently in-flight, so concurrent self-target deploys can race. | ||
| 4 | -- Fix design: catch the sqlite3.IntegrityError from the existing ux_deployment_active partial unique index and translate it into a new named ConcurrentDeploymentError (carrying git_commit_hash, blocking_deployment_id, blocking_status) instead of letting the raw sqlite error leak out. | ||
| 5 | -- Scope: only supervisor/deploy_stage.py (new exception class + one try/except) and supervisor/test_deploy_stage.py (tighten one existing test, add two regression tests for the rolled_back / blocked_pending_human terminal-state pass-through paths). No schema change, no new dependency, no app-level locking. | 3 | +对应 PRD:《`supervisor/deploy_stage.stage_deployment()` 并发部署冲突的诊断化处理》(FR-1~FR-5, AC-1~AC-6)。 |
| 6 | 4 | ||
| 7 | -git status in this worktree remains clean — the planner returned the SDD as its structured result rather than writing/committing it itself, consistent with graphlemon's artifact-commit flow happening at the orchestration layer. No further action taken; let me know if you want the full SDD text or want to proceed to the Coder step. | ||
| 8 | \ No newline at end of file | 5 | \ No newline at end of file |
| 6 | +## 1. 设计概述 | ||
| 7 | + | ||
| 8 | +本次改动不涉及 schema 变更（`ux_deployment_active` 唯一索引已存在，见 `src/schema.py:332-333`）， | ||
| 9 | +不引入新模块、不改变 `stage_deployment()` 的对外签名与返回值结构。改动局限于 | ||
| 10 | +`supervisor/deploy_stage.py` 一个模块内： | ||
| 11 | + | ||
| 12 | +1. 新增一个具名异常 `ConcurrentDeploymentError`（放置与文档风格对齐既有的 | ||
| 13 | + `CheckoutFailedError`/`deploy_cutover.CutoverFailedError`/`deploy_rollback.RollbackFailedError`）。 | ||
| 14 | +2. 在 `stage_deployment()` 触发 `INSERT INTO deployment ...` 的语句外包一层 `try/except | ||
| 15 | + sqlite3.IntegrityError`，把"因 `ux_deployment_active` 冲突导致的插入失败"精确识别出来并转译为 | ||
| 16 | + `ConcurrentDeploymentError`；非该冲突导致的 `IntegrityError` 原样重新抛出，不吞掉。 | ||
| 17 | +3. 一个新增只读诊断辅助函数，用于在转译时尽力查出当前占用中的部署行（`deployment_id`/`status`）， | ||
| 18 | + 供异常携带；查不到不阻塞异常抛出。 | ||
| 19 | +4. 测试：改造现有并发测试的断言粒度，并新增两个终态放行回归测试（`rolled_back`、`blocked_pending_human`）。 | ||
| 20 | + | ||
| 21 | +涉及文件： | ||
| 22 | + | ||
| 23 | +- `supervisor/deploy_stage.py`（修改） | ||
| 24 | +- `supervisor/test_deploy_stage.py`（修改 + 新增测试） | ||
| 25 | +- `src/schema.py`：**不改动**（`ux_deployment_active` 的终态集合定义本次不变，PRD 非目标已明确）。 | ||
| 26 | + | ||
| 27 | +## 2. 接口设计 | ||
| 28 | + | ||
| 29 | +### 2.1 新增异常类 `ConcurrentDeploymentError` | ||
| 30 | + | ||
| 31 | +放置位置：`supervisor/deploy_stage.py`，紧接在既有 `CheckoutFailedError`（第 38-42 行）之后，模块顶部， | ||
| 32 | +文档风格与其保持一致（触发场景 + 是否重试 + 已产生的副作用状态，均在 docstring 内说明）。 | ||
| 33 | + | ||
| 34 | +```python | ||
| 35 | +class ConcurrentDeploymentError(Exception): | ||
| 36 | + """CONCURRENT_DEPLOYMENT -- ux_deployment_active 唯一索引在插入新 deployment | ||
| 37 | + 行时检测到已存在一条未到终态（finalized/rolled_back/blocked_pending_human | ||
| 38 | + 之外）的部署记录。与 CheckoutFailedError 不同：这次 INSERT 从未落地提交， | ||
| 39 | + 没有半途而废的 deployment 行需要标记为 blocked_pending_human -- 调用方（Deploy | ||
| 40 | + Supervisor 轮询循环）应把这次调用视为"本轮跳过，下次轮询再试"，而不是 | ||
| 41 | + 需要人工介入的失败。诊断字段可通过属性访问，不只是拼进消息字符串： | ||
| 42 | + - git_commit_hash: 本次被拒绝、尝试发起的提交哈希。 | ||
| 43 | + - blocking_deployment_id / blocking_status: 当前占用中的部署记录（若能 | ||
| 44 | + 通过只读查询定位到）；查不到时二者均为 None，异常仍正常抛出。 | ||
| 45 | + """ | ||
| 46 | + | ||
| 47 | + def __init__(self, git_commit_hash, blocking_deployment_id=None, blocking_status=None): | ||
| 48 | + self.git_commit_hash = git_commit_hash | ||
| 49 | + self.blocking_deployment_id = blocking_deployment_id | ||
| 50 | + self.blocking_status = blocking_status | ||
| 51 | + message = "cannot stage commit {}: a deployment is already in progress".format(git_commit_hash) | ||
| 52 | + if blocking_deployment_id is not None: | ||
| 53 | + message += " (blocking deployment_id={}, status={})".format( | ||
| 54 | + blocking_deployment_id, blocking_status | ||
| 55 | + ) | ||
| 56 | + super().__init__(message) | ||
| 57 | +``` | ||
| 58 | + | ||
| 59 | +### 2.2 `stage_deployment()` 对外签名与返回值 | ||
| 60 | + | ||
| 61 | +不变：`stage_deployment(conn, git_commit_hash, repo_root=None, prod_db_path=None, | ||
| 62 | +checkout_parent_dir=None, port=None, python_executable=None)`，成功时返回值结构不变。 | ||
| 63 | +新增的唯一可观察差异是：冲突场景下抛出的异常类型从未声明的裸 `sqlite3.IntegrityError` | ||
| 64 | +变为具名的 `deploy_stage.ConcurrentDeploymentError`。 | ||
| 65 | + | ||
| 66 | +### 2.3 新增内部辅助函数 | ||
| 67 | + | ||
| 68 | +```python | ||
| 69 | +def _is_concurrent_deployment_conflict(error): | ||
| 70 | + """区分「ux_deployment_active 唯一约束冲突」与其他 IntegrityError（例如未来 | ||
| 71 | + schema 变化引入的其他约束、连接层面的完整性错误）。判别手段：sqlite 自 | ||
| 72 | + 3.8.2 起，对命名的 UNIQUE INDEX（区别于表列级 UNIQUE 约束）报错时会在 | ||
| 73 | + 异常消息中包含该索引名，形如 "UNIQUE constraint failed: index | ||
| 74 | + 'ux_deployment_active'"。只匹配这一具体索引名字符串，不笼统捕获所有 | ||
| 75 | + IntegrityError，避免把无关的完整性错误误判为并发冲突而掩盖真实问题。 | ||
| 76 | + """ | ||
| 77 | + return "ux_deployment_active" in str(error) | ||
| 78 | + | ||
| 79 | + | ||
| 80 | +def _find_blocking_deployment(conn): | ||
| 81 | + """只读诊断查询：定位当前占用 ux_deployment_active 的那一条非终态部署行。 | ||
| 82 | + 仅在检测到并发冲突之后调用，纯粹用于丰富异常的诊断信息。查询本身失败 | ||
| 83 | + （理论上不应发生，例如连接已损坏）不应掩盖原始的并发冲突 -- 这里吞掉查询期间 | ||
| 84 | + 的 sqlite3.Error 并返回 None，调用方在拿不到诊断信息时仍然正常抛出 | ||
| 85 | + ConcurrentDeploymentError，只是不携带 blocking_deployment_id/blocking_status。 | ||
| 86 | + """ | ||
| 87 | + try: | ||
| 88 | + row = conn.execute( | ||
| 89 | + "SELECT deployment_id, status FROM deployment " | ||
| 90 | + "WHERE status NOT IN ('finalized', 'rolled_back', 'blocked_pending_human') " | ||
| 91 | + "ORDER BY rowid DESC LIMIT 1" | ||
| 92 | + ).fetchone() | ||
| 93 | + return tuple(row) if row else None | ||
| 94 | + except sqlite3.Error: | ||
| 95 | + return None | ||
| 96 | +``` | ||
| 97 | + | ||
| 98 | +`import sqlite3` 需要新增到 `supervisor/deploy_stage.py` 顶部的标准库导入区（当前只导入了 | ||
| 99 | +`os, shutil, socket, subprocess, sys, uuid`）。 | ||
| 100 | + | ||
| 101 | +## 3. 控制流改造（`stage_deployment()` 内部） | ||
| 102 | + | ||
| 103 | +现状（第 170-176 行）： | ||
| 104 | + | ||
| 105 | +```python | ||
| 106 | +conn.execute( | ||
| 107 | + "INSERT INTO deployment " | ||
| 108 | + "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " | ||
| 109 | + "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", | ||
| 110 | + (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), | ||
| 111 | +) | ||
| 112 | +conn.commit() | ||
| 113 | +``` | ||
| 114 | + | ||
| 115 | +改造后（原地包一层 `try/except`，插入语句本身、参数、之后的 `_checkout_candidate` 起始点完全不变， | ||
| 116 | +满足 PRD FR-2 "不改变顺序，只改变冲突时的异常转译"约束）： | ||
| 117 | + | ||
| 118 | +```python | ||
| 119 | +try: | ||
| 120 | + conn.execute( | ||
| 121 | + "INSERT INTO deployment " | ||
| 122 | + "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " | ||
| 123 | + "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", | ||
| 124 | + (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), | ||
| 125 | + ) | ||
| 126 | + conn.commit() | ||
| 127 | +except sqlite3.IntegrityError as e: | ||
| 128 | + conn.rollback() | ||
| 129 | + if not _is_concurrent_deployment_conflict(e): | ||
| 130 | + raise | ||
| 131 | + blocking = _find_blocking_deployment(conn) | ||
| 132 | + blocking_deployment_id, blocking_status = blocking if blocking else (None, None) | ||
| 133 | + raise ConcurrentDeploymentError( | ||
| 134 | + git_commit_hash=git_commit_hash, | ||
| 135 | + blocking_deployment_id=blocking_deployment_id, | ||
| 136 | + blocking_status=blocking_status, | ||
| 137 | + ) from e | ||
| 138 | +``` | ||
| 139 | + | ||
| 140 | +`conn.rollback()` 的作用：sqlite3 默认 `isolation_level` 下，第一条 DML 语句会隐式开启一个事务； | ||
| 141 | +`INSERT` 抛出 `IntegrityError` 时该事务从未 `commit()`，`rollback()` 在这里是防御性的连接状态清理 | ||
| 142 | +（确保后续 `_find_blocking_deployment` 的只读查询、以及调用方复用同一个 `conn` 做后续操作时， | ||
| 143 | +连接处于干净状态），不是撤销任何已经落地的副作用——因为根本没有副作用落地。 | ||
| 144 | + | ||
| 145 | +之后的分支（`_checkout_candidate` 抛 `subprocess.CalledProcessError` -> `CheckoutFailedError`， | ||
| 146 | +第 178-206 行）完全不变，不受本次改动影响（对应 PRD AC-5）。 | ||
| 147 | + | ||
| 148 | +## 4. 异常传播路径总览 | ||
| 149 | + | ||
| 150 | +| 场景 | 触发点 | 抛出的异常 | `deployment` 表状态 | | ||
| 151 | +|------|--------|-----------|---------------------| | ||
| 152 | +| 已有非终态部署记录 | `INSERT` 语句触发 `ux_deployment_active` 冲突 | `ConcurrentDeploymentError`（携带 `git_commit_hash`/`blocking_deployment_id`/`blocking_status`） | 本次尝试的行从未写入（`INSERT` 失败即回滚），不新增任何行 | | ||
| 153 | +| 其他 `IntegrityError`（非本索引） | 同上 `INSERT` 语句，但错误消息不含 `ux_deployment_active` | 原始 `sqlite3.IntegrityError` 原样重新抛出 | 同上，不新增行 | | ||
| 154 | +| `INSERT` 成功，之后 checkout 失败 | `_checkout_candidate` 内 `git worktree add` 失败 | `CheckoutFailedError`（既有行为，不变） | 已插入的行被 `UPDATE ... SET status = 'blocked_pending_human'` | | ||
| 155 | +| `INSERT` 成功，checkout/db copy/launch 全部成功 | 无 | 无 | 行被 `UPDATE ... SET status = 'candidate_starting'` | | ||
| 156 | + | ||
| 157 | +## 5. 测试设计（对应 PRD FR-3/FR-4/FR-5，`supervisor/test_deploy_stage.py`） | ||
| 158 | + | ||
| 159 | +### 5.1 改造：冲突路径断言收紧（对应 AC-1，替换现有 `test_only_one_non_terminal_deployment_allowed`） | ||
| 160 | + | ||
| 161 | +```python | ||
| 162 | +def test_only_one_non_terminal_deployment_allowed(self): | ||
| 163 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | ||
| 164 | + mock_launch.return_value.pid = 1 | ||
| 165 | + first = deploy_stage.stage_deployment( | ||
| 166 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9001, | ||
| 167 | + ) | ||
| 168 | + with self.assertRaises(deploy_stage.ConcurrentDeploymentError) as ctx: | ||
| 169 | + deploy_stage.stage_deployment( | ||
| 170 | + self.conn, self.commit_v1, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9002, | ||
| 171 | + ) | ||
| 172 | + err = ctx.exception | ||
| 173 | + self.assertEqual(err.git_commit_hash, self.commit_v1) | ||
| 174 | + self.assertEqual(err.blocking_deployment_id, first["deployment_id"]) | ||
| 175 | + self.assertEqual(err.blocking_status, "candidate_starting") | ||
| 176 | + # 冲突时不应新增第二条部署记录 | ||
| 177 | + count = self.conn.execute("SELECT COUNT(*) FROM deployment").fetchone()[0] | ||
| 178 | + self.assertEqual(count, 1) | ||
| 179 | +``` | ||
| 180 | + | ||
| 181 | +注：第一次调用的 `mock_launch` 让 `stage_deployment()` 把行状态更新为 `candidate_starting` | ||
| 182 | +（第 195-200 行的既有逻辑），因此 `blocking_status` 断言为 `"candidate_starting"` 而非 `"staged"`。 | ||
| 183 | + | ||
| 184 | +### 5.2 新增：终态放行回归测试（对应 AC-3/AC-4，加入 `StageDeploymentHappyPathTest` 或独立测试类） | ||
| 185 | + | ||
| 186 | +```python | ||
| 187 | +class TerminalStateAllowsNewDeploymentTest(DeployStageTestBase): | ||
| 188 | + def _seed_deployment(self, status): | ||
| 189 | + self.conn.execute( | ||
| 190 | + "INSERT INTO deployment (deployment_id, git_commit_hash, previous_commit_hash, status, " | ||
| 191 | + "candidate_instance_dir, previous_instance_dir) " | ||
| 192 | + "VALUES ('dep-prior', 'commit-prior', 'commit-minus-1', ?, '/old/candidate/dir', '/older/dir')", | ||
| 193 | + (status,), | ||
| 194 | + ) | ||
| 195 | + self.conn.commit() | ||
| 196 | + | ||
| 197 | + def test_rolled_back_prior_deployment_allows_new_staging(self): | ||
| 198 | + self._seed_deployment("rolled_back") | ||
| 199 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | ||
| 200 | + mock_launch.return_value.pid = 111 | ||
| 201 | + result = deploy_stage.stage_deployment( | ||
| 202 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9101, | ||
| 203 | + ) | ||
| 204 | + self.assertEqual(result["status"], "candidate_starting") | ||
| 205 | + row = self.conn.execute( | ||
| 206 | + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) | ||
| 207 | + ).fetchone() | ||
| 208 | + self.assertEqual(row[0], "candidate_starting") | ||
| 209 | + | ||
| 210 | + def test_blocked_pending_human_prior_deployment_allows_new_staging(self): | ||
| 211 | + self._seed_deployment("blocked_pending_human") | ||
| 212 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | ||
| 213 | + mock_launch.return_value.pid = 222 | ||
| 214 | + result = deploy_stage.stage_deployment( | ||
| 215 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9102, | ||
| 216 | + ) | ||
| 217 | + self.assertEqual(result["status"], "candidate_starting") | ||
| 218 | + row = self.conn.execute( | ||
| 219 | + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) | ||
| 220 | + ).fetchone() | ||
| 221 | + self.assertEqual(row[0], "candidate_starting") | ||
| 222 | +``` | ||
| 223 | + | ||
| 224 | +### 5.3 覆盖矩阵 | ||
| 225 | + | ||
| 226 | +| PRD 验收标准 | 测试 | | ||
| 227 | +|---|---| | ||
| 228 | +| AC-1（并发冲突，具名异常 + 诊断字段 + 不新增行） | `test_only_one_non_terminal_deployment_allowed`（改造后） | | ||
| 229 | +| AC-2（`finalized` 放行，既有行为不回归） | `BootstrapPreviousTest.test_prior_finalized_deployment_supplies_previous_fields`（已存在，不改） | | ||
| 230 | +| AC-3（`rolled_back` 放行） | `test_rolled_back_prior_deployment_allows_new_staging`（新增） | | ||
| 231 | +| AC-4（`blocked_pending_human` 放行） | `test_blocked_pending_human_prior_deployment_allows_new_staging`（新增） | | ||
| 232 | +| AC-5（`CheckoutFailedError` 路径不回归） | `CheckoutFailedTest`（已存在，不改） | | ||
| 233 | +| AC-6（全量测试全绿） | `python3 -m unittest discover -s src` + `python3 -m unittest discover -s supervisor -p "test_*.py"` | | ||
| 234 | + | ||
| 235 | +## 6. Development Unit 分解 | ||
| 236 | + | ||
| 237 | +本次改动规模小、边界清楚、无跨模块依赖，符合 PRD 明确排除拆分的判断——分解为**一个** Development Unit。 | ||
| 238 | + | ||
| 239 | +### U-DS01：deploy_stage 并发冲突诊断化改造 | ||
| 240 | + | ||
| 241 | +- **类型**：Bug 修复（纵向、单模块，具备独立可验证产物）。 | ||
| 242 | +- **目标**：`stage_deployment()` 在遇到 `ux_deployment_active` 冲突时抛出携带诊断字段的具名异常 | ||
| 243 | + `ConcurrentDeploymentError`，并补齐终态放行的回归测试覆盖。 | ||
| 244 | +- **范围**： | ||
| 245 | + - `supervisor/deploy_stage.py`：新增 `import sqlite3`；新增 `ConcurrentDeploymentError` 类 | ||
| 246 | + （§2.1）；新增 `_is_concurrent_deployment_conflict`/`_find_blocking_deployment` 辅助函数（§2.3）； | ||
| 247 | + 改造 `stage_deployment()` 的 `INSERT` 语句为 `try/except`（§3）。 | ||
| 248 | + - `supervisor/test_deploy_stage.py`：改造 `test_only_one_non_terminal_deployment_allowed` | ||
| 249 | + 断言粒度（§5.1）；新增 `TerminalStateAllowsNewDeploymentTest` 两个用例（§5.2）。 | ||
| 250 | +- **不在范围内**：`src/schema.py`（不改）、`deploy_cutover.py`/`deploy_rollback.py`（不改， | ||
| 251 | + 仅作为命名风格参照）、`deploy_supervisor.py` 轮询循环对新异常的具体处理策略（PRD 未要求， | ||
| 252 | + 调用方目前对 `stage_deployment()` 的调用点若有裸 `except Exception` 兜底，`ConcurrentDeployment | ||
| 253 | + Error` 会被同样兜住，不需要额外改动；若需要针对性处理属于后续任务）。 | ||
| 254 | +- **接口契约**：`ConcurrentDeploymentError(git_commit_hash, blocking_deployment_id=None, | ||
| 255 | + blocking_status=None)`，属性只读、通过实例访问。`stage_deployment()` 签名与成功返回值结构不变。 | ||
| 256 | +- **依赖**：无跨 Unit 依赖，无外部依赖新增。 | ||
| 257 | +- **Done 标准**（可观察证据）： | ||
| 258 | + 1. `supervisor/deploy_stage.py` 中存在 `ConcurrentDeploymentError` 类定义，且 | ||
| 259 | + `stage_deployment()` 的并发路径抛出该类型而非裸 `sqlite3.IntegrityError`。 | ||
| 260 | + 2. §5.3 覆盖矩阵中列出的 6 个测试全部存在且通过。 | ||
| 261 | + 3. `python3 -m unittest discover -s src` 退出码 0。 | ||
| 262 | + 4. `python3 -m unittest discover -s supervisor -p "test_*.py"` 退出码 0。 | ||
| 263 | +- **风险**：低。唯一需要留意的实现细节是 §2.3 中 `_is_concurrent_deployment_conflict` 的字符串 | ||
| 264 | + 判别方式依赖当前 sqlite3 版本的错误消息格式（`"UNIQUE constraint failed: index '<name>'"`）—— | ||
| 265 | + 如果本地/CI 的 sqlite3 版本明显老旧导致消息格式不同，判别函数会退化为把该冲突当作"未知 | ||
| 266 | + IntegrityError"原样抛出（不会误判为其他类型的错误吞掉，只是不会转译成 | ||
| 267 | + `ConcurrentDeploymentError`）；`test_only_one_non_terminal_deployment_allowed` 断言的具体异常 | ||
| 268 | + 类型会在这种环境下失败并如实暴露该差异，而不是静默通过，因此不需要额外的运行期兜底代码。 | ||
| 269 | + | ||
| 270 | +## 7. 与既有约定的一致性检查 | ||
| 271 | + | ||
| 272 | +- 异常命名/放置/docstring 风格：对齐 `CheckoutFailedError`（同模块）、`CutoverFailedError` | ||
| 273 | + （`deploy_cutover.py`）、`RollbackFailedError`（`deploy_rollback.py`）——均为"具名、模块顶部、 | ||
| 274 | + docstring 说明触发场景与是否重试"。 | ||
| 275 | +- 不引入应用层锁：继续依赖 `ux_deployment_active` 数据库唯一约束作为并发正确性的唯一事实来源 | ||
| 276 | + （PRD 非目标已明确），本次只是把约束冲突的"表现形式"从裸异常变为诊断化异常，不改变并发正确性 | ||
| 277 | + 机制本身。 | ||
| 278 | +- 测试风格：延续 `supervisor/test_deploy_stage.py` 现有的"真实 sqlite + 真实 git worktree，只 mock | ||
| 279 | + `_launch_candidate`"约定，新增测试不引入额外 mock 面。 |