Repository navigation
fix(frontend): reuse authorization cache without stale grants - #29596
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? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep review of 5799fc3 against ff53ee2; main agent only, no subagents.
结论:本轮未发现需要 Request Changes 的问题。代码层面通过;CI 尚未结束,完整 TPCC 性能持平仍未证明。当前账号是 PR 作者,因此以 COMMENT 记录评审结论,不把它冒充 GitHub 的正式 APPROVE。
设计与实现:
- 权限决策继续归 frontend,缓存仍归现有 Session;disttae 只提供当前目录版本证明。没有新增持久化 epoch、广播器、后台线程或第二套权限状态机。
- 核对了七个目录依赖、物理表 ID、partition generation/applied watermark、快照下界传递、pending/reconnect/future-state 拒绝复用,以及缓存发布与后续变更的先后关系。未取得版本证明时清空缓存并走原授权 SQL;实际准入/订阅错误传播。
- 核对文本、SQL PREPARE/EXECUTE、二进制 PREPARE/EXECUTE、视图 definer 链、角色切换、会话身份重建、缓存开关与事务控制入口。EXECUTE 的 bound statement 仍独立授权;definer 的权限不会写入调用者缓存。
- 只读快照复用 TxnClient 的取消/关闭/释放机制。缓存只保留小型代际标识,正向动态 scope 上限 1024,负向查询不分配条目。删除的 set 方法和 hit/total 字段均无生产消费者。
额外 QA 验证(临时 Go overlay,无生产代码或提交修改):
- 真实双 CN,三个独立会话分别执行文本、SQL prepared、binary prepared。
- 补充跨库同名表/数据库级通配权限隔离、USE 后 prepared 绑定、失败 UPDATE 后缓存、继承角色 revoke/regrant。
- 补充全量恢复:远端三个连接先预热授权,再恢复到未授权的快照;三个连接均立即拒绝旧权限,再授权后恢复正常。
- 普通测试 PASS(11.773s);race 两轮 PASS(28.791s)。复用了本 head 对应的 frontend/disttae/logtailreplay 整包、原有恢复/PITR、事务取消/关闭及 SCA 有效证据。
增量分别审视:实现 +238/-82(净 +156),测试 +312/-88(净 +224),文档 +81。测试分别覆盖组件版本边界、容量与真实协议行为;未发现重复 fixture 或仅复制实现的新增测试需要阻塞本次提交。
性能证据支持显著减少重复授权 SQL:同构普通用户点查中位数 1413us -> 173us;相对无效化检查完全关闭的 unsafe 对照仍多约 17us。微基准及 profile 不能替代完整 TPCC,也不能据此承诺所有负载零退化。
评论核对:截至本轮检查,无人工 review/inline comments 待关闭;现有 Qodo 评论仅说明自动评审暂停。CodeQL、共享构建、Connector/J 已通过;Linux UT/SCA/coverage 与 BVT 仍在运行,尚无已报告失败。
fengttt
left a comment
There was a problem hiding this comment.
Do we have a TPCC run? I wonder what is the impact on TPMC.
XuPeng-SH
left a comment
There was a problem hiding this comment.
QA challenge of 5799fc3: 发现一个已归因的 P2 健壮性回归,代码评审结论更正为 REQUEST_CHANGES。此结论取代上一轮“无阻塞问题”的判断:#29596 (review) 。当前账号为 PR 作者,使用 COMMENT 记录阻断意见,非 GitHub 的正式 REQUEST_CHANGES 状态。
具体复现和根因见 inline comment:权限目录订阅失败时,普通用户及 accountadmin 的清缓存、关闭缓存和 SHOW WARNINGS 都无法执行;六个独立会话在干净 base 全过、head 全失败。
本轮其余挑战:
- 暂停授权读取于正向结果读出后、缓存发布前,让另一 CN 完成 REVOKE;权限撤销和角色成员关系撤销两种构型的下一条 SQL 均拒绝访问。普通测试通过;race 首轮 12.11s,据 B=30s 得到 N=2,随后两轮 12.50s/3.71s 均通过。
- 关闭版本检查的临时 unsafe 反例对照会在上述测试中出现撤权后访问仍成功,证明 oracle 可识别目标错误;该对照不会交付。
- 非管理员 definer 的嵌套视图(显式授权中间视图)在文本、SQL prepared、binary prepared 三个独立缓存下通过;访问外层视图没有赋予调用者直接访问中间视图/底层表的权利,远端撤销 definer 的底层 SELECT 后三个入口均拒绝访问。
- 未显式授权中间视图的初始构型,两版本均在 PREPARE 阶段拒绝,未归为本 PR 新回归。
- 订阅重建的 shared-state identity、pending/readiness,以及 scope 容量边界复用了未变更的组件/race 证据,并重新追查其实际 reset/GC 所属路径;未发现第二个阻断问题。
本轮只用了外部 overlay 与独立干净 base worktree,未修改生产代码、测试提交或 PR head,也未开 subagent。
有 |
PR 就是修复TPCC 性能回退 |
What type of PR is this?
Which issue(s) this PR fixes:
Follow-up to #29532; preserves the authorization and restore fixes covered by #29399. Rebased onto latest main
ff53ee2eb0c(includes #29466).What this PR does / why we need it:
#29532 invalidates session privileges at every statement so remote REVOKE and RESTORE cannot reuse stale grants. That also turns warm TPCC executions into repeated privilege SQL, planning and compilation. This change reuses the existing privilege cache only after proving its authorization catalogs are unchanged at a fresh authorization snapshot.
Validation
pkg/frontend,pkg/vm/engine/disttae,pkg/vm/engine/disttae/logtailreplay— PASS.-race— PASS. Rebase only changes IN-list narrowing/index backfill in the planner, so unchanged authorization/lifecycle race evidence is reused.TestIssue29399*snapshot/PITR/ownership tests andTestIssue27834CrossCNAuthenticationReadsLatestCatalog— PASS after rebase.Follow-up validation
pkg/frontend: PASS. Incremental configured SCA (frontend,tests/issues): 0 issues.Performance and limits
Historical measurements at pre-follow-up revision
5799fc3b227, using a same-base local public-protocol probe: one row, three rounds of 1,500 prepared point reads and 300 BEGIN / SELECT FOR UPDATE / UPDATE / COMMIT transactions per user. Median microseconds:A separate same-workload profile on the rebased code shows cumulative sampled allocation under
authenticateUserCanExecutePrepareOrExecutefalling from about 8,136 MiB to 58 MiB; sampled CPU there falls from 12.90s to 0.02s (the latter is near sampling resolution). These are authorization-path measurements, not overall throughput ratios.The unsafe control only estimates historical reuse cost and is not delivered. Ordinary point reads improve about 88% versus per-statement clearing but retain about 17us over that control. Transaction times are near the control; this shared macOS machine and small workload do not establish unchanged full TPCC performance. Full TPCC comparison remains required to claim parity. The evidence-only probe/overlays/profiles are excluded from the patch.
Follow-up timing checks compared the prior revision with consumer-owned freshness, in both run orders and then ABBAAB within one cluster/connection. They did not show a stable attributable regression: standalone point reads were around 143–152us, but transaction results and some interleaved point samples varied substantially. These noisy local samples do not establish full TPCC parity. The structural cost check above establishes that ordinary hot paths add no freshness probe and retain plan/Compile reuse. Prepared EXPLAIN EXECUTE deliberately rebinds its nested handle to preserve authorization. A final interleaved stage probe showed lower locked-SELECT/UPDATE means for the consumer-owned check, while ordinary COMMIT mean/tail increased (median roughly 5.66ms vs 5.78ms). This narrows the local timing uncertainty but still does not prove full-workload equivalence. Representative TPCC performance acceptance remains pending.
Production: +254/-90 (net +164). Tests: +468/-120 (net +348), reusing existing fixtures. Documentation: +93/-0. See
docs/design/20261004-authorization-cache-reuse.md.