Skip to content

Add production-grade online logs slimming job - #686

Draft
jjcc123312 wants to merge 3 commits into
mainfrom
codex/logs-slimming-job
Draft

Add production-grade online logs slimming job#686
jjcc123312 wants to merge 3 commits into
mainfrom
codex/logs-slimming-job

Conversation

@jjcc123312

@jjcc123312 jjcc123312 commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Problem / background

The production logs table has reached roughly 100M rows with data and indexes both around 60 GB. Root-user Codex traffic accounts for most growth and is now routed to logs_company, but historical rows still need an online, customer-transparent compaction path.

Direct bulk DELETE or an unmanaged table rebuild would create unacceptable undo/redo, replication, metadata-lock, disk, and rollback risk on the shared Cloud SQL instance.

Evidence and reproduced risks

Staging exercises were run only against the isolated newapi_staging schema on the shared Cloud SQL instance. The job hard-refuses the production newapi schema.

Observed and covered failure modes include:

  • interrupted backfill resumed from a durable checkpoint;
  • a concurrent second job was rejected by the database advisory lock;
  • DDL watchdog interrupted a blocked DDL in 2,435 ms with a 3 s ceiling;
  • atomic cutover and rollback ran with concurrent retained/filtered writers (71 successful, 0 failed);
  • rollback was interrupted after RENAME during trigger transfer, then recovered from persisted topology intent;
  • filtered-row cleanup removed 29 rows written directly to the compact table by the fault harness;
  • Threads_running=19 exceeded the configured threshold of 16 and stopped the operation before topology mutation;
  • final snapshots showed no retained-row loss, filtered-row leakage, or marker duplication.

Design / scope

This PR adds an independent operational binary and build definition:

  • cmd/logs_slimming: preflight, prepare, bounded backfill, forward trigger, reconcile, verify, cutover, rollback, recover, and cleanup commands;
  • durable checkpoint generation/CAS and a database advisory lock for multi-node/job safety;
  • exact schema, database user, host, server UUID, GCP project/instance, channel-ID, table fingerprint, and trigger ownership checks;
  • bounded batches, statement timeouts, load thresholds, full-field verification, AUTO_INCREMENT reservation, MDL barrier, tagged DDL watchdog/postconditions, and fail-closed UNKNOWN outcomes;
  • atomic two-table RENAME for cutover and rollback;
  • staging-only Cloud Build / Cloud Run Job image.

No application router, console, frontend, Terraform, Cloudflare, or production migration path is changed.

Impact and risks

The code is isolated from the serving application and is invoked only as a standalone Job. The main remaining risk is operational misuse or database load during a future production migration; production therefore remains NO-GO and requires a separately reviewed production-enabled artifact and runbook.

The checked-in binary currently rejects schema=newapi; it must not be weakened or reused directly for production.

Multi-node correctness does not depend on process memory: coordination uses MySQL advisory locking, durable checkpoint CAS, database triggers, and atomic RENAME.

Validation

Fresh after rebase onto current main:

  • gofmt -l cmd/logs_slimming — clean
  • go vet ./cmd/logs_slimming — pass
  • go test -race -count=1 ./cmd/logs_slimming — pass
  • go build -o /tmp/logs-slimming-evaluator ./cmd/logs_slimming — pass
  • GitNexus compare against origin/main — 12 isolated added files, low risk, no existing execution flows affected
  • independent review — P0: 0, P1: 0; staging controlled rollback/recovery GO; production NO-GO
  • evaluator — PASS: code gates and staging 1-9 evidence are present

Staging evidence covers preflight, resumable backfill, advisory locking, append-only guards, DDL watchdog, AUTO_INCREMENT barrier, concurrent cutover writers, topology recovery, non-root rollback API behavior, and production zero-write audit.

Non-root rollback API acceptance

A real non-root staging session (test_codex) validated the post-rollback user log page:

  • /api/log/self — Cloud Run HTTP 200 at 2026-08-11T08:49:48.742232Z
  • /api/log/self/stat — Cloud Run HTTP 200 at 2026-08-11T08:49:48.735794Z
  • UI returned 73 rows, page 1/1, usage $0.000246
  • no query error and no matching endpoint HTTP 5xx in the two-hour audit window
  • evidence contains no password, session cookie, or token

The non-root acceptance item passed for commit bb6de4d0c, but commit e9b9df997 changes trigger and watchdog behavior; the prior runtime evidence is therefore historical only. PR remains Draft until the updated artifact completes the full staging 1–9 suite again.

Deployment recommendation

  • Router deploy: not required
  • Reason: all changes are isolated to a standalone operational binary and its dedicated build files; no relay, billing, middleware, shared runtime initialization, or serving schema is changed.
  • Other deploy targets: no console, web, Terraform, or Cloudflare deployment. Only the separately controlled staging Cloud Run Job is in scope.
  • Risk / validation: production remains NO-GO. Any future production execution requires a separately built production artifact, low-traffic runbook, live Cloud SQL monitoring, abort thresholds, independent review, and explicit approval.

@jjcc123312

jjcc123312 commented Aug 11, 2026

Copy link
Copy Markdown
Author

Final staging acceptance is complete.

Evidence:

  • Real non-root staging session (test_codex) loaded the post-rollback usage-log page successfully: 73 rows, page 1/1, usage $0.000246.
  • Cloud Run request logs show GET /api/log/self = HTTP 200 at 2026-08-11T08:49:48.742232Z.
  • Cloud Run request logs show GET /api/log/self/stat = HTTP 200 at 2026-08-11T08:49:48.735794Z.
  • A two-hour read-only Cloud Logging audit found no matching endpoint 5xx.
  • rollback-api-final.jsonl records only sanitized acceptance metadata; it contains no password, cookie, or token.
  • Final evaluator: PASS: code gates and staging 1-9 evidence are present.
  • GitNexus compare against origin/main: 12 added files, low risk, no existing execution flows affected.
  • GitHub pr-quality: pass; merge state: clean.

Scope remains unchanged: staging GO; production NO-GO. The checked-in binary still hard-refuses the production newapi schema. Production requires a separate artifact, runbook, review, and explicit authorization.

@KingCesc

KingCesc commented Aug 11, 2026

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 commit bb6de4d0 · 共 15 条

cmd/logs_slimming/ddl.go

  • L130: [严重] 这里使用 maxTime 会把后置观测截止时间延长到 DDL deadline 之后，导致 runDDL 的总耗时可能超过 c.ddlTimeout；这与函数注释中“bounds the complete operation”以及 ddl-timeout 作为 DDL wall watchdog 的语义不一致，线上流水线可能在认为已受硬超时保护时额外阻塞。建议要么让观测使用不晚于 deadline 的上下文，要么把 observation grace 显式纳入配置/命名并在调用方可见。
observeDeadline := deadline
  • L135-138: [严重] DDL 已发出并记录 ddl_intent 后，如果打开 observer 失败会直接返回，缺少对应的 ddl_postcondition 终态证据；生产审计/恢复时无法区分 PRE、POST 还是 UNKNOWN，削弱 fail-closed 语义。建议在 observer 打开失败时也写入 result=unknown，并带上错误原因、exec_resolved、kill_error 和已观测 states 后再返回。
if openErr != nil {
			cancel()
			if emitErr := ev.emit("ddl_postcondition", map[string]any{"operation": operation, "nonce": nonce, "statement_sha256": statementHash, "state": fmt.Sprint(states), "result": "unknown", "exec_resolved": resolved, "kill_error": killErr, "observe_error": openErr}); emitErr != nil {
				return fmt.Errorf("write observer-open DDL postcondition evidence: %w", emitErr)
			}
			return openErr
		}
  • L142-144: [严重] DDL 已发出并记录 ddl_intent 后，如果 observe 返回错误会直接返回，缺少 ddl_postcondition 终态证据；这会让后续 recover/人工排查只能看到 intent 而没有 UNKNOWN 结论。建议在 observe 错误路径先 emit result=unknown，再返回原始观测错误。
if observeErr != nil {
			if emitErr := ev.emit("ddl_postcondition", map[string]any{"operation": operation, "nonce": nonce, "statement_sha256": statementHash, "state": fmt.Sprint(states), "result": "unknown", "exec_resolved": resolved, "kill_error": killErr, "observe_error": observeErr}); emitErr != nil {
				return fmt.Errorf("write observe-error DDL postcondition evidence: %w", emitErr)
			}
			return observeErr
		}
  • L150-155: [严重] 只要 killErr 非空就返回 UNKNOWN，会误伤一种常见竞态：watchdog 触发时 KILL 证明失败/目标已结束，但 ExecContext 随后成功返回且两次后置观测都稳定为 POST。此时实际 DDL 已成功，当前逻辑仍中断 logs slimming 流程，可能导致不必要的人工恢复或重复执行。建议先判定稳定 POST 且 execErr==nil 的成功场景，将 killErr 作为 evidence 中的 warning 字段记录；只有执行未解决、执行失败或观测不稳定时再返回 UNKNOWN。
if !resolved {
		if err := ev.emit("ddl_postcondition", map[string]any{"operation": operation, "nonce": nonce, "statement_sha256": statementHash, "state": fmt.Sprint(states), "result": "unknown", "exec_resolved": resolved, "kill_error": killErr}); err != nil {
			return fmt.Errorf("write unresolved DDL postcondition evidence: %w", err)
		}
		return fmt.Errorf("DDL outcome UNKNOWN: exec_resolved=%t kill=%v states=%v", resolved, killErr, states)
	}

cmd/logs_slimming/evidence.go

  • L37-47: [阻塞] 这里仅在写入 record 时对顶层 string/error 做脱敏，map、slice、struct、[]byte 或自定义 Stringer 等嵌套值会被 json.Encoder 直接序列化为原始内容；一旦调用方把包含 DSN、密码或 token 的嵌套对象放进 fields，evidence 日志会泄漏敏感信息。建议对字段做递归脱敏，并在 JSON 编码后再对最终输出做一次 secret 替换兜底。
for key, value := range fields {
		record[key] = e.redactValue(value)
	}
	var buffer bytes.Buffer
	encoder := json.NewEncoder(&buffer)
	encoder.SetEscapeHTML(false)
	err := encoder.Encode(record)
	data := bytes.TrimSuffix(buffer.Bytes(), []byte{'\n'})
	if err != nil {
		data = []byte(fmt.Sprintf(`{"timestamp":%q,"event":"evidence_marshal_failed","error":%q}`, time.Now().UTC().Format(time.RFC3339Nano), e.redactString(err.Error())))
	} else {
		data = []byte(e.redactString(string(data)))
	}

cmd/logs_slimming/sqlgen.go

  • L0: [阻塞] 这里的脱敏只覆盖顶层 string/error，其他类型会原样交给 JSON 编码；一旦 evidence 字段中传入包含 DSN、密码或 token 的嵌套 map/slice/struct、[]byte 或 fmt.Stringer，证据日志会直接落盘/输出敏感信息。该工具会记录线上变更证据，建议实现递归脱敏并在最终 JSON 字节流写出前再做一次兜底替换，确保任何字段形态都不会绕过 secret 过滤。
+func (e *evidence) redactValue(value any) any {
+	switch typed := value.(type) {
+	case error:
+		return e.redactString(typed.Error())
+	case string:
+		return e.redactString(typed)
+	case []byte:
+		return e.redactString(string(typed))
+	case fmt.Stringer:
+		return e.redactString(typed.String())
+	case map[string]any:
+		redacted := make(map[string]any, len(typed))
+		for key, nested := range typed {
+			redacted[key] = e.redactValue(nested)
+		}
+		return redacted
+	case []any:
+		redacted := make([]any, len(typed))
+		for i, nested := range typed {
+			redacted[i] = e.redactValue(nested)
+		}
+		return redacted
+	default:
+		return typed
+	}
+}

cmd/logs_slimming/ops.go

  • L79-81: [严重] 这里遍历完 channels 结果集后只调用 Close，没有检查 rows.Err()；如果迭代过程中连接中断或服务端返回中途错误，currentChannelIDs 可能是不完整快照，但后续仍会用它做冻结 channel 校验，导致预检误判或迁移错误中止。建议在关闭前显式检查 rows.Err() 并返回错误。
if err := rows.Err(); err != nil {
		rows.Close()
		return fmt.Errorf("iterate Codex channel snapshot: %w", err)
	}
	if err := rows.Close(); err != nil {
		return fmt.Errorf("close Codex channel snapshot: %w", err)
	}
  • L468-471: [严重] schemaFingerprint 的 rows.Next() 循环结束后未检查 rows.Err()，迭代中途失败时可能基于不完整的列/索引/分区元数据生成指纹并让一致性校验误通过或误失败。该函数是迁移前后 schema 冻结校验的核心，建议每个 Rows 循环结束后先检查 rows.Err()，再关闭结果集。
if err := rows.Err(); err != nil {
		rows.Close()
		return "", err
	}
	if err := rows.Close(); err != nil {
		return "", err
	}
	rows, err = db.QueryContext(ctx, "SELECT INDEX_NAME,NON_UNIQUE,SEQ_IN_INDEX,COALESCE(COLUMN_NAME,'<NULL>'),COALESCE(COLLATION,'<NULL>'),COALESCE(SUB_PART,-1),NULLABLE,INDEX_TYPE,COALESCE(INDEX_COMMENT,''),IS_VISIBLE,COALESCE(EXPRESSION,'<NULL>') FROM information_schema.statistics WHERE table_schema=? AND table_name=? ORDER BY INDEX_NAME,SEQ_IN_INDEX", schema, table)

cmd/logs_slimming/config.go

  • L114-118: [严重] 这里把运行 schema 永久限制为 newapi_staging 并拒绝 newapi，但本次新增的构建配置/Dockerfile 没有看到用于生产的单独构建参数或替换机制；如果该镜像用于“production-grade online logs slimming job”，生产 preflight/cutover/rollback/recover 等核心流程会在配置校验阶段全部不可执行。建议将允许的 schema 通过受控的编译期变量或独立生产构建注入，并在生产构建中仍保持显式 allowlist/身份校验，而不是在通用代码里固定拒绝生产 schema。
allowedSchema := stagingSchema
	if buildSchema != "" { // 由受控的生产/预发构建在编译期注入，例如 -ldflags "-X main.buildSchema=newapi"
		allowedSchema = buildSchema
	}
	if c.schema != allowedSchema {
		return fmt.Errorf("schema %q is denied; this artifact only permits %q", c.schema, allowedSchema)
	}

cmd/logs_slimming/topology.go

  • L83-91: [严重] cleanupPlan 只根据对象拓扑和 ownershipConfirmed 放行清理，没有校验当前触发器拓扑。调用方在 PRE 拓扑下即使存在异常/遗留的 reverse trigger，也可能拿到清理计划并继续删除 target/checkpoint，而 reverse trigger 留在 live source 上后续会引用不存在或错误的对象，导致线上 INSERT 失败或镜像一致性被破坏。建议把 triggerTopology 纳入计划输入，并在清理前明确要求不存在 reverse trigger、forward 与 guard 都是预期且挂在 live source 上。
func cleanupPlan(status topologyStatus, triggers triggerTopology, ownershipConfirmed bool) ([]cleanupStep, error) {
	if status != topologyPreCutover {
		return nil, fmt.Errorf("cleanup only supports stable pre-cutover topology, got %s", status)
	}
	if !ownershipConfirmed {
		return nil, fmt.Errorf("cleanup ownership is not confirmed")
	}
	if triggers.reverse || !triggers.forward || !triggers.updateGuard || !triggers.deleteGuard {
		return nil, fmt.Errorf("cleanup requires exact pre-cutover trigger topology, got %+v", triggers)
	}
	return []cleanupStep{cleanupDropForward, cleanupDropGuards, cleanupDropTarget, cleanupDropCheckpoint}, nil
}

cmd/logs_slimming/main.go

  • L70-78: [严重] 这里把除 preflight/prepare 外的所有命令都统一绑定到 checkpoint 加载和冻结指纹校验，可能阻断故障恢复类操作。尤其是 recover/rollback/cleanup 的价值就在于处理 cutover/rollback 中断、checkpoint 处于 intent 状态或对象拓扑异常的场景；如果 checkpoint 缺失/损坏、表刚被 RENAME 导致指纹校验暂时不满足，这些命令会在进入各自恢复逻辑前直接失败，导致无法执行清理或回滚。建议只对 backfill/reconcile/install-forward-trigger/verify/cutover 等依赖冻结指纹的正常流程做该前置校验，把 recover/rollback/cleanup 的前置条件下放到各自实现中按拓扑和阶段判断。
switch c.command {
	case "backfill", "reconcile", "install-forward-trigger", "verify", "cutover":
		state, err := loadCheckpoint(ctx, db, c)
		if err != nil {
			return err
		}
		if err := assertFrozenTableFingerprints(ctx, db, c, state); err != nil {
			return err
		}
	}

cmd/logs_slimming/cutover.go

  • L970-975: [阻塞] RENAME 完成后，原先的 update/delete guard 会随旧 live 表一起移动到 old，新的 live 表在这里创建 guard 之前存在未保护窗口；同样 reverse mirror 也是在 DROP forward 之后才创建。生产流量在该窗口执行 UPDATE/DELETE 时不会被触发器阻断，后续 reconcile 主要按 INSERT/ON DUPLICATE 补洞，无法可靠恢复已删除或已变更的 live 行，可能导致切换后数据丢失或回滚数据不一致。建议在 RENAME 前通过全局写入闸门/短暂停写覆盖稳定阶段，或设计能随目标表原子生效的 guard/mirror 方案，确保 live 表在拓扑切换全过程始终被 UPDATE/DELETE guard 保护。
if len(actions) > 0 {
		// 在进入该分支前必须已经通过外部写入闸门/停写机制保证 live 表无 UPDATE/DELETE/INSERT 竞态，
		// 或改为预置可随 RENAME 原子生效的 guard/mirror 触发器方案。
		if err := assertWriteGateClosed(ctx, db, c); err != nil {
			return err
		}
		_, t, err = observeTopology(ctx, db, c)
		if err != nil || t.forward || t.reverse {
			return fmt.Errorf("forward drop postcondition does not prove zero mirror triggers: triggers=%+v err=%v", t, err)
		}
		query, err := buildStrictMirrorTriggerSQL(c, c.source, c.old)
  • L341-344: [严重] 如果 evidence 写入失败会直接返回，但此时 ddlConn 已打开且没有 defer 兜底关闭；多次重试或临时证据存储故障会泄漏 MySQL 会话/连接池资源，可能导致后续 DDL、观测或业务连接受影响。建议在 ddlConn 创建后立即注册带状态的 defer，确保所有早退路径都会 Close；只有在 DDL goroutine 已启动且需要后台回收时再解除该 defer。
ddlStarted := false
	defer func() {
		if !ddlStarted {
			_ = ddlConn.Close()
		}
	}()
	if err := ev.emit("rename_barrier_intent", map[string]any{"nonce": nonce, "connection_id": ddlFacts.connectionID, "statement_sha256": fmt.Sprintf("%x", sha256.Sum256([]byte(statement)))}); err != nil {
		return topologyUnknown, true, err
	}
	done := make(chan error, 1)
	ddlStarted = true
  • L477-481: [严重] 这里要求 PROCESSLIST.INFO 与完整 tagged SQL 精确相等，同时 metadata_locks 中只能出现 barrier/DDL 两类 owner。线上普通查询可能短暂持有兼容的 GRANTED MDL，PROCESSLIST.INFO 在不同配置下也可能被截断，都会被误判为 pending proof failed 并触发 abort，导致在线 cutover 在真实流量下频繁不可用。建议改为基于 connection id + nonce/comment 前缀或 digest 的稳健匹配，并对业务连接的短暂兼容锁采用等待收敛/白名单策略；若必须排他校验，应明确在代码中先确认维护窗口或停读停写。
default:
			// 不要因业务查询短暂持有的兼容 GRANTED MDL 立即失败；应继续等待其释放，
			// 或在进入 cutover 前显式校验维护窗口/停读停写闸门。
			return fmt.Errorf("unexpected MDL owner=%d status=%s", owner, status)
		}
	}
	if err := rows.Err(); err != nil {
  • L282-286: [严重] 仅通过 SHOW CREATE TABLE 正则提取 AUTO_INCREMENT=N 会在空表或 next value 仍为默认值的表上失败，因为 MySQL 可能不输出该表选项；这会阻断空日志表、刚初始化 target 或 rollback 表的 cutover/rollback。建议改用 information_schema.tables.AUTO_INCREMENT，并在 NULL 时用 COALESCE(MAX(id),0)+1 兜底处理空表场景，再保留 ALTER 后的 readback 校验。
match := autoIncrementPattern.FindStringSubmatch(create)
	if len(match) == 2 {
		return strconv.ParseUint(match[1], 10, 64)
	}
	// SHOW CREATE 可能在空表/默认自增值时省略 AUTO_INCREMENT；请从 information_schema.tables.AUTO_INCREMENT
	// 读取，并在 NULL 时回退到 SELECT COALESCE(MAX(id),0)+1。
	return informationSchemaAutoIncrementOrMaxNext(ctx, q, schema, table)

@jjcc123312

jjcc123312 commented Aug 11, 2026

Copy link
Copy Markdown
Author

Follow-up on the final review gate:

  • The evaluator now parses every evidence file with jq and enforces the acceptance semantics, rather than checking only file presence.
  • Required assertions now include both self endpoints at HTTP 200, non-root UI rows > 0, matching 5xx = 0, credentials_recorded=false, writer failures = 0, retained missing = 0, filtered leakage = 0, marker duplicates = 0, DDL watchdog < 3000 ms, production hard-deny = true, and production DDL/DML/deploy = false.
  • Fresh semantic evaluator result: PASS: code gates and staging 1-9 evidence are present.
  • Goal state and ledger now record validation_passed at 2026-08-11T08:58:43Z.
  • Sanitized evidence SHA-256:
    • rollback-api-final.jsonl: e0a36ad9c3483d700ff98bb24c6d0ab880760028c1cac02638f65c7d0480bcab
    • production-zero-write.json: 57de2740c902817e749be84a84d823c2f1c3b073cf090f021747b9d66d83a92b
    • semantic evaluate.sh: 05d536b05402b22409270f60ba8d45afb781ee40b4c4855826bd1d5bc6eca7c8

The PR body already marks the prior non-root acceptance item complete. Scope remains staging GO / production NO-GO; no production resource was changed.

@jjcc123312

jjcc123312 commented Aug 11, 2026

Copy link
Copy Markdown
Author

Resolved the actionable findings from OpenCodeReview comment #5251079323 in commit e9b9df997.

Resolution by finding:

  1. runDDL exceeded its wall watchdog — fixed. Postcondition observation now uses the original DDL deadline; it cannot extend beyond --ddl-timeout.
  2. Observer-open failure lacked terminal evidence — fixed. It emits ddl_postcondition with result=unknown, observed states, execution resolution, kill error, and observation error.
  3. Observer-query failure lacked terminal evidence — fixed through the same fail-closed UNKNOWN path.
  4. Stable POST was rejected solely because KILL proof raced — fixed. A resolved execution plus two identical exact POST observations is accepted; kill_warning remains in evidence. Unresolved or unstable outcomes remain UNKNOWN.
  5. Nested evidence values could bypass redaction — fixed. Redaction recursively handles structs, maps, slices/arrays, pointers/interfaces, strings, byte slices, errors, and Stringers before JSON encoding, with serialized-output replacement as a final backstop. Tests include a struct secret containing quotes and backslashes.
  6. Duplicate redaction finding in sqlgen.go — covered by the same recursive evidence fix.
  7. Channel snapshot omitted rows.Err() — fixed; iteration and close errors now fail preflight.
  8. Schema fingerprint omitted rows.Err() — fixed for columns, indexes, and partitions.
  9. Production schema build switch suggestion — intentionally not applied. This PR is a staging-only artifact and hard-denying newapi is a reviewed safety boundary. A production-enabled artifact requires a separate PR, runbook, and explicit authorization; adding an ldflags escape hatch here would weaken that boundary.
  10. Cleanup ignored trigger topology — fixed. Cleanup now receives observed trigger topology and refuses any reverse trigger; each existing owned trigger is still verified against its exact spec before deletion. It intentionally permits partial PRE setup so a failed prepare can be cleaned safely.
  11. Recover/cleanup fingerprint precheck suggestion — intentionally not applied. Recovery remains fail-closed when the checkpoint is missing/corrupt or the frozen schema identity is untrusted. Normal RENAME transitions preserve the table fingerprint and were exercised in staging recovery tests.
  12. RENAME left the new live table temporarily without UPDATE/DELETE guards — fixed. Separately named future guards are installed on the compact target before cutover and move with it atomically during RENAME. The original guards remain with the old table, so rollback is also protected immediately. UPDATE guards allow only a full-row no-op required by ON DUPLICATE KEY UPDATE id=id; all real mutations and all DELETEs are rejected.
  13. Early evidence failure leaked the RENAME DDL connection — fixed with a pre-goroutine close defer guarded by ddlStarted.
  14. Exact pending-Rename proof was too strict — intentionally retained. The code polls until compatible transient readers drain; it does not abort on the first reader. It uses information_schema.PROCESSLIST.INFO, connection identity, nonce/tag, advisory-lock owner, and exact MDL ownership. Relaxing this would weaken the final ID/AUTO_INCREMENT barrier proof.
  15. Empty/default table may omit AUTO_INCREMENT in SHOW CREATE — fixed. The code falls back to information_schema.tables.AUTO_INCREMENT, then MAX(id)+1 when NULL, with overflow checking.

Additional correction from independent review:

  • Future UPDATE guards originally conflicted with internal duplicate no-op reconciliation. They now compare all 21 columns with NULL-safe equality and signal only on a real mutation; forward and rollback-gap no-op paths are covered by tests.
  • Both generic DDL and RENAME settle/observation paths now honor the same hard deadline and emit UNKNOWN evidence when the result cannot be proven.

Fresh validation on e9b9df997:

  • gofmt — clean
  • go vet ./cmd/logs_slimming — pass
  • go test -race -count=1 ./cmd/logs_slimming — pass
  • go build ./cmd/logs_slimming — pass
  • git diff --check — pass
  • GitNexus compare against origin/main — 12 isolated files, low risk, no existing execution flows affected (the new standalone command is not yet symbol-indexed)
  • Independent final review — P0: 0, P1: 0, P2: 2 non-blocking

The PR remains Draft. Previous staging evidence belongs to bb6de4d0c; the updated commit must complete the full staging 1–9 suite, including real MySQL trigger/RENAME behavior, before this PR can become Ready. Production remains NO-GO and no router/console/web/Terraform/Cloudflare deployment is required.

@KingCesc

KingCesc commented Aug 11, 2026

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 增量 bb6de4d0..e9b9df99 · 共 7 条

cmd/logs_slimming/ddl.go

  • L141: [严重] 这里把 postcondition 观测截止时间限制为 DDL 总 watchdog 的同一个 deadline；如果 DDL 在接近超时时刚成功返回，后续两次只读确认可能没有任何时间窗口，会直接上报 UNKNOWN 并中断 logs slimming 流程。建议为“执行 watchdog”和“执行完成后的只读确认”保留独立且受控的宽限期，避免成功 DDL 因无法观测而被误判。
observeDeadline := deadline
	if graceDeadline := time.Now().Add(ddlObservationGrace); graceDeadline.After(observeDeadline) {
		observeDeadline = graceDeadline
	}

cmd/logs_slimming/evidence.go

  • L107-130: [严重] 这里只对指针做了循环检测，map/slice 通过 interface 自引用时不会进入 seen，map[string]any{"self": m} 或 []any{s} 会在 evidence 输出时无限递归直到栈溢出，导致 logs_slimming 任务崩溃。建议对 map/slice 也加入访问栈，或设置最大递归深度并在超过阈值时输出占位符。
case reflect.Slice, reflect.Array:
		if value.Type().Elem().Kind() == reflect.Uint8 {
			bytes := make([]byte, value.Len())
			for i := range bytes {
				bytes[i] = byte(value.Index(i).Uint())
			}
			return e.redactString(string(bytes))
		}
		// TODO: 对 slice/map 等引用类型也记录访问栈，或引入最大递归深度，避免 interface 自引用导致无限递归。
		items := make([]any, value.Len())
		for i := range items {
			items[i] = e.redactReflect(value.Index(i), seen)
		}
		return items
	case reflect.Map:
		if value.IsNil() {
			return nil
		}
		// TODO: 对 map 记录访问栈，检测 map[string]any 等动态对象的自引用循环。
		items := make(map[string]any, value.Len())
		iterator := value.MapRange()
		for iterator.Next() {
			key := e.redactString(fmt.Sprint(iterator.Key().Interface()))
			items[key] = e.redactReflect(iterator.Value(), seen)
		}
		return items
  • L131-152: [严重] 反射展开结构体会绕过类型自定义的 MarshalJSON/json.Marshaler 逻辑，原本依赖自定义序列化隐藏或裁剪的导出字段会被写入 evidence；最终字符串替换只能覆盖已登记 secret，无法防止未登记敏感字段泄漏。建议在反射展开前优先识别并尊重 json.Marshaler，或改为基于类型自己的 JSON 输出做脱敏。
case reflect.Struct:
		// TODO: 在手动展开结构体前优先尊重 json.Marshaler/MarshalJSON，避免绕过类型自身的序列化脱敏/裁剪逻辑。
		items := make(map[string]any)
		typ := value.Type()
		for i := 0; i < value.NumField(); i++ {
			fieldType := typ.Field(i)
			field := value.Field(i)
			if !fieldType.IsExported() || !field.CanInterface() {
				continue
			}
			name := fieldType.Name
			if tag := fieldType.Tag.Get("json"); tag != "" {
				parts := strings.Split(tag, ",")
				if parts[0] == "-" {
					continue
				}
				if parts[0] != "" {
					name = parts[0]
				}
			}
			items[name] = e.redactReflect(field, seen)
		}
		return items

cmd/logs_slimming/ops.go

  • L276-280: [阻塞] 这里会把 future_guard_delete 安装到 compact/target 表；cutover 后它会随表变为 live source，保护是有效的，但在 rollback RENAME 后会再次随表回到 c.target。当前回滚恢复流程随后需要对 c.target 执行 filtered rows DELETE（removeFilteredRollbackRows），该 BEFORE DELETE 触发器会拦截这一步，导致回滚无法完成并卡在 rollback-reconcile。建议在回滚 PRE 清理前对 c.target 上的 future_guard_delete 做受控 drop，清理完成后再重建；或调整回滚步骤顺序/触发器逻辑，确保内部清理不会被 append-only guard 阻断。
futureKind := "future_guard_" + event
		futureQuery, err := buildNamedGuardTriggerSQL(c, futureKind, event, c.target)
		if err != nil {
			return err
		}
		// 回滚路径需要在删除 c.target 上的 filtered rows 前受控移除 future_guard_delete，
		// 并在清理完成后重建；否则该触发器会阻断 rollback-reconcile。
  • L276-280: [阻塞] installForward 会再次确保 c.target 上存在 future_guard_delete；该触发器在正常切换后保护新 live 表，但一旦执行 rollback，它会随 RENAME 回到 compact target，并阻断后续 rollback-reconcile 对 c.target 的 DELETE 清理，导致回滚核心流程不可用。建议配套修改回滚恢复流程：在删除 filtered rows 前先校验并 drop c.target 上的 future_guard_delete，清理结束后再按期望 spec 重建，或提供安全的内部清理绕过机制。
futureKind := "future_guard_" + event
		futureQuery, err := buildNamedGuardTriggerSQL(c, futureKind, event, c.target)
		if err != nil {
			return err
		}
		// 注意：回滚到 PRE 后，future_guard_delete 会位于 c.target；
		// rollback-reconcile 删除 filtered rows 前必须先受控移除并在完成后恢复。

cmd/logs_slimming/cutover.go

  • L393-400: [严重] 这里把后置拓扑观测的截止时间收紧为总 DDL deadline。进入 abort 分支时前面已经经历 pending/kill/rollback 等操作，deadline 很可能已过期或只剩极短时间，observeStableObjectTopology 会立即失败并把实际已完成的 RENAME 判为 unknown，导致恢复/回滚流程进入人工介入状态。建议保留一个独立的最小观测窗口（同时不缩短原有更晚的 deadline），确保 DDL 结果能被稳定确认。
observeDeadline := time.Now().Add(750 * time.Millisecond)
		if deadline.After(observeDeadline) {
			observeDeadline = deadline
		}
		status, topologyErr := observeStableObjectTopology(db, c, observeDeadline)
		result := "known"
		if topologyErr != nil || status == topologyUnknown || !resolved {
			result = "unknown"
		}
		if evidenceErr := emitPostcondition(result, status, resolved, cause, topologyErr); evidenceErr != nil {
			return topologyUnknown, resolved, fmt.Errorf("%w; rename postcondition evidence: %v", cause, evidenceErr)
		}
  • L458-464: [严重] 正常释放 MDL 后也直接复用总 deadline 做拓扑确认；如果 DDL 刚在超时边界返回成功，后续观测可能因为 deadline 已到而失败，进而把成功 cutover 误报为 unknown。建议与异常路径一致，为后置观测保留独立 grace window，避免成功 RENAME 被误判。
observeDeadline := time.Now().Add(750 * time.Millisecond)
	if deadline.After(observeDeadline) {
		observeDeadline = deadline
	}
	status, topologyErr := observeStableObjectTopology(db, c, observeDeadline)
	if topologyErr != nil {
		if evidenceErr := emitPostcondition("unknown", topologyUnknown, resolved, execErr, topologyErr); evidenceErr != nil {
			return topologyUnknown, resolved, fmt.Errorf("rename=%v topology=%v evidence=%w", execErr, topologyErr, evidenceErr)
		}
		return topologyUnknown, resolved, fmt.Errorf("rename=%v topology=%w", execErr, topologyErr)
	}

@KingCesc

KingCesc commented Aug 11, 2026

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 增量 e9b9df99..35efe976 · 共 1 条

cmd/logs_slimming/cutover.go

  • L721-725: [阻塞] 这里在 defer 恢复 future DELETE guard 时继续复用调用方传入的 ctx；如果前面的清理因为任务取消、超时或优雅退出触发 context canceled/deadline exceeded 返回，defer 内的 runDDL 会基于已取消的父 ctx 创建 opCtx，DDL 很可能无法执行，导致此前已 DROP 的删除保护触发器无法恢复。该 guard 用于防止 compact target 被误删，线上中断场景可能留下不完整 PRE 拓扑。建议在 guard 被删除后，恢复逻辑使用保留 evidence/lockOwner 等必要 value 的独立短超时 context（或进入不可取消关键区），并在恢复失败时走明确的 fail-safe/告警/重试路径。
defer func() {
		restoreCtx, cancel := context.WithTimeout(context.Background(), c.ddlTimeout)
		defer cancel()
		if ev, ok := ctx.Value(evidenceContextKey{}).(*evidence); ok {
			restoreCtx = context.WithValue(restoreCtx, evidenceContextKey{}, ev)
		}
		if owner, ok := ctx.Value(lockOwnerContextKey{}).(int64); ok {
			restoreCtx = context.WithValue(restoreCtx, lockOwnerContextKey{}, owner)
		}
		query, err := buildNamedGuardTriggerSQL(c, "future_guard_delete", "delete", c.target)
		if err == nil {
			err = runDDL(restoreCtx, db, c, query, exactTriggerObserver(guardSpec, true))
		}

@jjcc123312

jjcc123312 commented Aug 11, 2026

Copy link
Copy Markdown
Author

Staging follow-up for commit 35efe9764:

  • Root cause confirmed: rollback filtered-row cleanup was blocked by the compact target's unconditional future_guard_delete trigger (MySQL 1644).
  • Fix: only during rollback-reconcile, on the inactive compact target, the Job drops the exact owned future DELETE guard, performs filtered cleanup, and recreates the guard with the persisted definer and SQL mode before leaving the phase. Other phases remain fail-closed; live logs guards are never removed.
  • Local validation: go test -race -count=1 ./cmd/logs_slimming, go vet ./cmd/logs_slimming, go build ./cmd/logs_slimming, and git diff --check passed. Independent review found P0=0/P1=0; the remaining P2 is real-MySQL crash-timing coverage.
  • Staging real-MySQL validation: the previously stuck rollback-reconcile recovered to fresh; final verify execution newapi-staging-logs-slimming-pp2w4 passed. All four guards had the expected table, definer newapi_staging_app@%, and persisted SQL mode. test_codex (user 119) returned HTTP 200 from both /api/log/self and /api/log/self/stat.
  • Safety evidence: the Job correctly stopped when Threads_running=33 exceeded the configured threshold 16. Final staging service check showed 100% traffic on a Ready revision, HTTP 5xx=0, severity ERROR=0, and lock-wait timeout hits=0 for the observed window.
  • Cleanup execution newapi-staging-logs-slimming-d4xnn passed. Final topology: source logs=1, temporary tables=0, migration triggers=0, Threads_running=1. Proxy and temporary helpers were removed.

The attempted cancel-after-guard-drop injection completed before the cancellation API took effect, so that exact SIGKILL timing is not claimed as passed. Per the decision to stop expanding staging fault permutations, the PR remains Draft and production remains NO-GO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jjcc123312 @KingCesc