Skip to content

perf(plan): cache bounded legacy TEXT syntax evidence - #31008

Open
LeftHandCold wants to merge 1 commit into
matrixorigin:mainfrom
LeftHandCold:codex/legacy-text-parse-cache-20261024
Open

LeftHandCold wants to merge 1 commit into
matrixorigin:mainfrom
LeftHandCold:codex/legacy-text-parse-cache-20261024

Conversation

@LeftHandCold

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

Fixes #31007

What this PR does / why we need it:

Repeated plan validation on mixed TEXT/LONGTEXT tables reparses the same stored CREATE under 64 SQL modes. Reuse successful immutable syntax evidence in a bounded 32-slot cache. Keep current table identity, Seqnum, column compatibility and CREATE LIKE transaction/snapshot resolution live on every call. Errors and oversized inputs retain the original uncached path.

The cache owns detached SQL/name strings and column slices, at most 2 MiB of payload plus fixed headers. Parsing and allocation stay outside the lock; lookup/publication inspect at most 32 entries. No AST, table definition, transaction, goroutine or unbounded miss registry is retained.

On 998,799 complete AgentLogs records with the optional STORED layout, causal A/B/B/A (ten measured samples per version/query) changes Q03 from 8.206 to 0.810 ms, Q05 from 114.367 to 106.958 ms, and Q09 from 27.056 to 19.390 ms. Original-layout controls remain within 1.4% wall variation. All complete results/native metadata/physical identities pass; candidate full20 checks pass on both layouts. Four CPU cores, whole-engine/client/validator 12 GiB, no swap; measured peak 5.899 GiB and zero OOM/limit events. This isolates the cache on b787 plus the identical Group fix; it excludes later CONCAT and combined-patch gains.

Validation:

  • Sixteen existing/new focused tests pass on baseline and candidate: detached ownership/exact keys, capacity boundaries/eviction, concurrent access, live ALTER/Seqnum/LIKE checks, errors/ambiguity and fallback. A borrowed-key mutation fails the ownership assertion.
  • Full owning tests, scoped vet/lint, native provenance, full owning race and adaptive concurrent race repetitions pass. Warm-recovery microbenchmarks improve substantially; cold, uncached and oversized controls show no observed slowdown.
  • Clean delivery head 267d535d1f on main 1651eec94875: full plan-package tests and four existing frontend table-dump recovery/schema/manifest consumers individually pass. Scoped vet/lint/gofmt/native provenance pass. Evidence: legacy-text-cache-delivery-main-closure-r1.json, SHA e924f8d906820e8f0f8c9ecd6565e7538765dcf7af15cffd0e7f4f79934db772.

This draft changes three plan-package files only. It includes no benchmark layout, Group fix, schema format or storage migration. The separate Group prerequisite is needed by the benchmark fixture, not by this cache implementation.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@LeftHandCold LeftHandCold left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

代码复核:建议通过,未发现需要 Request Changes 的确定性问题。审查 HEAD:267d535d1ff53d1d91cba6f6617b881ec17a1bd6。

设计判断:这是现有 legacy TEXT 恢复路径的局部解析性能修复,不是新增 schema 恢复能力或重构 catalog 所有权。缓存只记住确定性的语法证据,原解析器和恢复校验继续是唯一语义实现;无需另建通用缓存框架。对新增进程级状态单独审查了所有权、并发和边界。

逐项核对:

  • exact SQL 为 key;解析仍枚举原有 SQL modes。仅成功解析入缓存,错误/歧义不缓存,超出限额则回到原解析路径。
  • SQL/名称字符串和列切片由缓存复制持有,不保留 AST、TableDef、事务或解析器借用内存;现有消费者只读 evidence。
  • 当前表身份、Seqnum、列兼容性、CREATE LIKE 的当前数据库/事务 resolver 每次重新检查。因此 DROP/ALTER/新快照不会直接复用旧 catalog 结论。
  • 32 槽、SQL 16KiB、256 列、单条 64KiB 限制,保留 payload 至多 2MiB 加固定头部;读者持有的旧 entry 由普通 Go 引用生命周期释放。解析/分配在锁外,锁内仅有限数组查找/发布,没有后台 worker 或无界 miss registry。
  • 测试包含 ownership、并发发布/淘汰、live LIKE resolver、identity/Seqnum、防歧义和超限回退。冷 miss 仍支付原 64-mode 解析成本,这是明确的取舍,不是所有 schema 场景都会加速。

验证边界:当前 HEAD 的 CI 40038653492 已成功,并核对其 UT 日志的 planner race 分片完成;diff --check 通过。本次未在本机重跑 planner/AgentLogs,PR 的真实数据收益是作者证据。

当前认证账号也是 PR 作者,故以 COMMENT 记录建议通过;这不是 GitHub 正式 Approval。

@XuPeng-SH

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · reviewing

Commit: 267d535d1ff53d1d91cba6f6617b881ec17a1bd6

Review in progress.

This branch was successfully deployed

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

Labels

do-not-merge/wip kind/enhancement size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Performance]: Repeated legacy TEXT declaration parsing adds milliseconds to short queries

3 participants