perf: reduce bootstrap, metadata and buffer allocation costs - #29589
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
4d7eec4 to
9546880
Compare
XuPeng-SH
left a comment
There was a problem hiding this comment.
评审通过:已审查 commit 3c3a52f20aa1a688363e11b1c4afb3a32b3bc0e4 的全部 33 个变更文件及相关生产入口、调用者和生命周期。审查范围以 merge-base 6249e488b51f9b6887e8db95ce9bfc92b4eb49e9 为起点(PR declared base 为 8daed14755ff13a81356085455cd14facb61049f)。当前实现未发现阻塞问题。
设计与正确性:优化落在已有职责内,没有引入重复执行器、状态机或资源所有者。初始化权限批处理保留原事务、权限字段和失败中止语义;恢复路径删除不可达的手工 CREATE fallback,同时保留 sequence、UDF、外键和 view 所需元数据。恢复注解扫描保持原 RE2 语言,并修复空解析结果访问。HAKeeper 的轻量状态投影仅供状态断言使用,完整状态消费者保持独立快照。分片并行度复用现有 CPU 预算、线程池和 spill 约束;事件、对象 buffer 与 segment 的容量调整保留原有清理、重试和 transport 回调职责。
已挑战取消、错误传播、关闭、重试、池复用和缓冲区别名路径:没有发现新的等待环或无界全局保留。保留的单行权限格式、完整状态查询及资源释放路径仍有实际消费者,不属于退役遗漏。
验证:当前 head 的 16 个定向测试通过;恢复注解与对象名分类的差分 fuzz 分别执行 14,142 和 23,906 次并通过。精确 head 的 MatrixOne ALL CI 成功,包含适用的构建、UT/race、SCA、coverage 和 BVT。未将跳过的检查视为通过。没有未处理的实质性 review comment。
增量分别核算:实现 +161/-134(净 +27),测试 +1108/-62(净 +1046),文档 +121/-3(净 +118)。逐项检查了新增测试的行为断言和 fixture 复用,未以测试占比替代设计审查。
性能结论有边界:已有本地串行 race 对比节省 163.14 秒,但样本有外部任务干扰,内存峰值和部分 UPDATE 时序未一致改善,且测量早于最终测试整理。当前结果足以接受这些有明确 SQL/分配收益的局部优化,不能证明实际 CI 节省十分钟,也不能宣称所有场景性能提升。父 issue #29562 的性能目标应继续保持开放。
本次未使用 subagent。PR 作者与提交账号均为 XuPeng-SH,GitHub 不支持自我 APPROVE,因此以 COMMENT 提交正式评审通过结论。
What type of PR is this?
Which issue(s) this PR fixes:
Related to #29562. CI wall-time savings remain to be measured at the checked PR SHA.
What this PR does / why we need it:
Embedded race tests repeatedly exercise real bootstrap, restore, metadata classification and stream/object writers. Reduce their common owner costs while retaining the original serial runner, GC policy, test selection and lifecycle boundaries. Bootstrap emits one privilege INSERT per fixed role; restore relies on the existing CLONE schema/data owner and drops dead manual-create fallbacks. State-only HAKeeper assertions omit unused cluster snapshots. Object-name and restore-annotation classifiers preserve the old RE2 languages with stateless scans. Branch defaults use the existing CPU budget; event, logtail segment and object entry buffers allocate for actual payloads. Expression traversal reuses the struct field count without a cache.
The prepared EXPLAIN test also uses the live authorization SQL owner after main removed its old forwarding helper; SQL and authorization assertions are unchanged.
Validation before the subsequent test-only consolidation: both matched current-base/head canonical stages passed, with the same 651 test pass/skip and package terminal identities. All ten modified owner packages passed vet and incremental configured lint; make err-check passed. Full frontend race passed after the final annotation refinement; full owning/dependent normal/race evidence and cancellation/error/retry/cleanup, typed-nil/detached-state, byte-roundtrip and independent classifier oracles cover the changed contracts. Privilege catalog checks add coverage inside an existing real cluster fixture.
Matched ten-package serial race stage on local base 6249e48:
Go1.26.4, race/short/matrixone_test, UTC, GOMAXPROCS=8, affinity 0–15, CPU quota 800%, memory limit 16GiB, default GC, isolated HDD temp storage. Precompiled binaries used the byte-verified original serial runner and unchanged test selection. No competing owned compilation/test; foreign jobs were observed in both runs. These are local observations, not precise quiet measurements or confirmed CI savings; no peak-memory reduction is claimed. Older fcd9686 profiles are excluded from this gain calculation. Both local whole-issues attempts exhausted the selected 20m diagnostic budget (base 121/144 and head 128/144 top-level PASS); incomplete profiles are excluded from performance claims. The same 23 complete remaining top-level tests passed on both binaries. Prefix plus tail covers the identical 144 top-level tests and all their subtests; this is not a single-process whole-package PASS or a CI timeout change.
Additional resource checks retain mixed outcomes:
All supplemental scopes recorded zero OOM or memory-limit events and remained below about3.3GB charged memory under the existing 16GiB limit. The engineering conclusion is lower observed total CPU/resident occupancy for the ten-package stage, with mixed local boundaries and adequate measured capacity; the stronger quiet/all-peaks resource gate is not claimed passed. Actual CI gains and the original issue's 10min target remain unconfirmed.
Tradeoffs: reused small logtail segments pay about 19ns for the capacity guard in the deterministic normal warm benchmark. Event overflow above two args remains supported but the three-arg normal benchmark is about 67% slower with an extra allocation; current production call sites fit the inline budget. Wide 300-column/four-block object writes measured about0.95% slower under race with lower allocation bytes. No admission/scheduling/GC experiment is delivered. One clean-main MinIO setup timeout was preserved as invalid measurement; focused/full/original-runner controls passed without a retry/skip/deadline change.
Test/document consolidation: shared event growth/cleanup coverage across log/discard paths; one explicit cluster-cleanup matrix preserves system-only behavior and both ordered DELETE failures; object writer cases share MemoryFS/mpool with distinct paths and scoped cleanup. Decoded column count excludes vacuous value checks. The 800-case RE2 differential matrix and fuzz cases remain; benchmark timing no longer contains a self-derived correctness oracle. Six design documents become one owner-contract document. Net diff falls from 1,350 to 1,191 lines (production +27, tests +1,046, documentation +118). Final affected normal/race tests and scoped vet/lint/err-check are rerun after consolidation. Existing stage/profile figures above predate this consolidation; no additional CI savings are attributed to it.