test(perf): establish PendingRequestTable pressure and recovery matrix - #577
Conversation
a8d4fb7 to
a7caaca
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
The matrix is well-scoped and the final-head CI/P1 evidence chain is otherwise convincing, but I found one correctness-of-evidence blocker in the recovery heap measurement. The harness owner retains every completed request ID for its entire lifetime, so the full-GC heap series includes monotonic test-only retention. That makes the reported retained-heap range unsuitable for closing #571's repeated-overload heap-stability criterion. Please make the exactly-once bookkeeping bounded/resettable at a point where no completion can still arrive (or otherwise exclude that harness state from the heap evidence), then rerun the recovery/P1 evidence. Minor provenance cleanup: doc/pending-request-matrix.md says the production profile uses Brotli, while ConfigureProductionCompression installs SharpLinkZstdCompressionProvider; please make those agree.
cbca01f to
104cc43
Compare
bc6b454 to
e365284
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
继续复审后,前一轮两个 blocker(recovery heap harness retention、Brotli/Zstd provenance)都已经正确修复,当前 CI/P1 evidence 也一致。但在把 #571 视为可关闭前,我还看到一个组合覆盖缺口:production-shaped P1 cells 并没有实际把 PendingRequestTable 推到高占用,也没有 feature-heavy 里要求的 cancellation/deadline mix。
#571 的核心问题之一是回答“pending 高占用在 TLS/codec/telemetry/retry/admission/breaker 等生产配置下是否仍然可见/是否放大 saturation”。当前 RunProductionProfileAsync 的 typical/feature-heavy 都只是 8/16 个并发的成功 RPC;正式 P1 artifact 中 typical 的 pendingHighWater 是 8、feature-heavy 是 16,retries=0、resourceExhausted=0,并且该 profile 没有注入 cancellation/deadline。这样只能建立低占用 production baseline,不能回答 issue 里要求的 cross-feature saturation 问题。分开的 high-occupancy micro cells 和 long/short deadline cells也不能替代,因为它们没有走 TLS/compression/metrics/tracing/retry 的真实 RPC 路径。
建议至少增加一个 targeted P1 production-shaped saturation cell:用 deterministic server/request barrier 持有足够多真实 RPC,把 actual pending occupancy 推到明确目标(例如 90%/99%,或一个有理由的较高可控值),然后在 TLS + Zstd + metrics/tracing + retry/admission/breaker 下运行短请求/受控释放;如果 #571 的 feature-heavy contract继续保留 cancellation/deadline mix,也应在这个或另一个 targeted production cell 中覆盖。报告 actual occupancy/high-water、P99、retry/rejection/error 分布并保持现有 zero-pending/reuse hard gate。若不想把这一组合纳入 #577,则 Closes #571 应暂时去掉并把这部分留给后续 evidence。
7b53b29 to
d913021
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
继续复审当前 head d9130215a4de9d476fb8b257597fc20b38d77d2a:没有发现新的矩阵 correctness 问题,但上一轮的 production-shaped saturation blocker 仍然存在。当前 PendingRequestMatrixProductionProfiles.RunProductionProfileAsync 仍只跑 8/16 并发的成功 RPC;没有 deterministic held-open/high-occupancy phase,也没有 feature-heavy contract 中的 cancellation/deadline mix,因此仍不能用 Closes #571 回答“高 pending occupancy 与 TLS/Zstd/metrics/tracing/retry/admission/breaker 组合后的 saturation/tail 行为”。
此外,dev 已从该 PR 的 base d03ef7ed... 前进到 fd8832ce...,新提交是 #594 runtime retry-policy generation;它直接改动 SharpLinkClient.Retry.cs/runtime retry capture,而本 PR production-shaped profiles 明确开启 retry。当前 head 的 Fast/Pending+codec/CodeQL/Extended 虽然都绿,但这些 evidence 都是旧 retry implementation/base 上取得的。请先 restack 到当前 dev, 再补前述 targeted production saturation/cancel-deadline cell,并在最终 clean head 上重跑 CI/P1 evidence。
d7e8c6d to
257dadb
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
复审后,上一轮的 production-shaped saturation blocker 已经实质补齐,当前 base/head 与 P1 provenance 也干净。现在只剩一个 evidence correctness blocker:报告和 PR body 声称“same-session reuse”,但实现只在 recovery 后发了一次成功 QuickAsync,没有比较 session/connection identity;隐藏的 reconnect 仍会让这个 probe 通过。对 #571 这种恢复/连接稳定性证据,sessionReuse=true 不能是常量。请在 saturation service 暴露当前 SharpLinkCallContext session id(或等价的物理连接 identity),在 barrier 前/恢复后分别采样并断言一致;如果只想证明 client 可继续使用,则把字段/描述改成 clientReusable,不要声称 same-session。除此之外本轮没有新的 blocker。
ec19d68 to
a8c91cd
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
当前实现复审通过:之前的 recovery heap、Zstd provenance、production-shaped saturation,以及 same-physical-session reuse evidence 都已实质修复;sessionReuse 现在比较 server-side SharpLinkCallContext.Current.SessionId,P1 也记录到前后完全相同的 session identity。当前 exact-head Fast / Pending+codec / CodeQL / Padding / Extended 都有成功记录,没有新的 matrix correctness blocker。
但 #580 刚刚 squash merge 后,dev 已从本 PR 的 base 3bccdefd75fbb1f7e04ad6816f615d198004230e 前进到 a4ec60dda401fe50b07b82902d2786cb14d1290a,而 #577 仍停在旧 base,GitHub 当前显示 mergeable=false / compare behind 1, ahead 1。这次 dev 增量只有 #580 的 extension-fault workflow/doc/3 个 integration-test tracker 文件,没有改变 PendingRequestTable 或 production retry/admission/telemetry 实现,所以不推翻本轮代码结论;但最终合并前仍请把 #577 的单个 tracker commit restack 到当前 dev,并在新的 exact head 上重新确认 required CI。若要把 P1 artifact 作为最终关闭 #571 的严格 provenance,建议也在最终 restack head 上重跑/重新记录 P1。
Closes #571
Scope
Establishes a reproducible
PendingRequestTableperformance/stability evidence matrix without adding benchmark-only instrumentation to the production hot path.Matrix
TimeProviderscans plus real-timer lateness evidenceCorrectness gates
Review follow-up
RecordingOwnerdrops its exactly-once request-IDHashSetat quiescent zero, so full-GC recovery evidence does not retain a monotonically growing harness-only history.SharpLinkZstdCompressionProvider.pending-validation.ymltrigger now coversSharpLinkTelemetry*.cs,PendingRequestTable.cs,SharpLinkClient*.cs, andSharpLinkCircuitBreaker*.cs, so telemetry/pending ownership plus runtime timeout/retry/admission/circuit-breaker/heartbeat partial lifecycle changes cannot bypass the matrix.devadvanced through runtime timeout/retry/circuit-breaker/admission/heartbeat work; final evidence is on currentdevfeat(client): update heartbeat configuration at runtime #598 rather than an older lifecycle implementation.feature-heavy-saturation: TLS + Zstd + metrics + client/server tracing + retry + circuit breaker + admission control, a 64-entry pending table, 58 deterministically held calls (90.625% occupancy), a 64-call short burst, two explicit cancellations, and two method deadlines.SharpLinkCallContext.Current.SessionIdon the server; a successful post-recovery probe records it again. The matrix computessessionReusefrom exact string equality and hard-fails on any session change, so a hidden reconnect cannot satisfy the gate.Final stack
Base
dev:3bccdefd75fbb1f7e04ad6816f615d198004230e(#598 runtime heartbeat configuration).Final clean head:
a8c91cd8ce9129d321fc9edd0b06768a56055012.The PR is exactly one commit / eight files on top of current
dev.Final-head validation
34192202381: PASS, including [perf-test][client] 建立 PendingRequestTable 高占用、稀疏 deadline、长短混合与过载恢复矩阵 #571 CI matrix10042607431, digestsha256:b194c5b0901cc0575fb6f664662d6c6c859b9d54f3bc0ca11b7eac223d05a8faa8c91cd8ce9129d321fc9edd0b06768a56055012tier=ci, 23 cells, all invariants true34192202267: PASS — allocation, formatting, maintainability, Release build, generated-reference guard, Unit/Generator/Load tests34192202263: PASS34192202256: PASS on Linux, macOS, and WindowsFormal P1 review evidence
P1 was rerun through temporary workflow-only head
ec19d68f5d67c45c4fd428f41d85252e81d63bc1, then removed from PR history. Comparing clean heada8c91cd...to that temporary head shows exactly one changed file,.github/workflows/pending-validation.yml, with 2 additions / 2 deletions (step label plusci -> p1); implementation/test files are identical.34192038264: PASS10042554250, digestsha256:38904a338aae8566dc8e506f472de44ab9676f692199a95812b6fdd6db7d4b6btier=p1, 122 cells, all invariants truefeature-heavy-saturationevidence:sessionBefore = 9908afe2433c4995bef4174831620c51sessionAfter = 9908afe2433c4995bef4174831620c51sessionReuse = trueReady-for-review validation
34192513758: PASS — Debug/Release builds, admission rollback, full Integration Tests, NativeAOT transport/topology smoke, pack, NuGet contract/package smoke, Demo Oneway, Load Smoke, and all desktop codec produce/verify/summary jobs.34192497180: PASS on rerun. Its first attempt had one unrelatedSharedMemoryPipelineLifecycleTests.RejectedSecondReadShouldNotBreakTheActiveReadNotificationrace assertion; the same Fast job was immediately rerun and the complete Unit/Generator/Load suite passed, matching the already-green earlier exact-head Fast run.