Repository navigation
feat(compression): add dynamic runtime policy control - #553
Conversation
SunSi12138
left a comment
There was a problem hiding this comment.
整体方向基本对:instance-scoped local send-policy snapshot、窄化的 Response preference wire intent、handshake 初值、cumulative ACK、O(1) in-flight/latest,以及 compression hot path 在 rent/provider 前完成 binding/policy/preference/size 判断,这些都符合 #550 最终收敛目标。
但我认为现在还不能把 #550 视为完整收口,原因不只在 correctness:
- inline 有一个真实的 handshake -> Ready publication race,会让新 session 永久停在旧 response preference;这个需要修并补 deterministic race coverage。
- inline 的 build/runtime model 还少收了一层:runtime authority 已经拆开,但
SharpLinkCompressionOptions仍长期承载 threshold,并由它创建初始 send policy;这和 #550 最终 addendum 里“CompressionOptions 只剩 Providers / directional policy 独立”的模型不一致,也没有干净的 build-timeEnabled=falsedirectional initial policy。 - inline 的 cohort propagation 对第一个 live send failure 是 fail-fast;desired state 已经 publish,但后面的 cohort session 连 reconcile 都不会执行,生产环境一次 send-queue congestion 会留下额外的 stale Ready sessions。
- 验收证据还不够。当前 Fast / Extended / CodeQL 都是 green,但 Extended 只有 load smoke,没有 #550 明确要求的 no-provider / negotiated-disabled / enabled / after-update hot-path A/B;PR 里新增测试也集中在 protocol/session 单元层,没有覆盖 public Client/Server API 的 fixed cohort、reconnect/Ready-publication race,以及 Request/Response/ClientStreaming/ServerStreaming/DuplexStreaming 的 dynamic policy integration。这个 PR 改的是每个 outbound business frame 的路径,建议合并前把性能和 lifecycle/integration evidence 补齐,而不是用全量测试通过代替针对性验收。
除了这些点,provider/profile immutable、inbound decode 独立、ACK 非 wire barrier、stale generation 不回滚等核心状态机目前看是合理的,不建议再扩大抽象。
e9712e2 to
1377c4f
Compare
d120a8a to
1ab0846
Compare
1ab0846 to
6ccc33d
Compare
1ab0846 to
6ccc33d
Compare
|
Review follow-up is complete on current head Final acceptance evidence:
Current-head PR Fast, CodeQL and Codec Padding Security Evidence are all green. All review threads are resolved. Ready for review. |
SunSi12138
left a comment
There was a problem hiding this comment.
复审当前 head 后,第一轮三个问题都确认闭合:Ready publication race 已用 publication 后 reconcile + deterministic 三 topology test 修掉;capability / directional policy 已真正拆开;fixed cohort propagation 不再因前部 live failure 放弃后续 session。四种 RPC shape、reconnect、build-time disabled/same-session enable,以及当前 hot-path evidence 也已经补齐到足够支持这个架构方向。
现在只剩两个值得收口的点,见 inline。第一个是 public cancellation 语义的实际 bug,建议修后补一个 2-session cancellation test;第二个是交付/可用性问题:旧 threshold API 已删除,但用户文档还没有告诉调用方新的 directional builder/runtime API 和 response preference 怎么配置。
性能方面,runner 已经实际记录 allocation / CPU/op / elapsed/op / QPS,而且 no-provider 分支在代码上仍然先于 policy snapshot load;我不建议为了这一处 pointer-load/predictable-branch 再扩大实现。P50/P99 若最终 evidence 流程已有统一输出可继续记录,但我不把它作为这轮代码 blocker。
6ccc33d to
39f980c
Compare
6ccc33d to
39f980c
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
最终复审当前 head 39f980c268632f7469bf2a9c5601e212c9a02c9e:没有发现新的 blocking issue,可以合并。
确认:
- Ready-publication race、capability/policy ownership、fixed-cohort fail-fast 三个首轮问题均已闭合;
- caller cancellation 已恢复标准
OperationCanceledException语义,并有 2-session regression; - runtime compression 文档已补齐 Capability / RequestPolicy / ResponsePolicy / ResponsePreference 四层模型;
- local policy 继续是 instance-shared immutable snapshot,per-message hot path 无 session fan-out / lock / allocation;no-provider 在 policy snapshot load 前返回;
- remote preference 保持 cumulative ACK + one in-flight/latest pending 的 O(1) bounded state;
- fixed/static/dynamic Ready-gap、reconnect、build-time disabled -> same-session enable、unary/client-streaming/server-streaming/duplex、backpressured cohort failure 等 targeted coverage 已具备;
- 当前 head PR Fast / CodeQL / Codec Padding Security Evidence 均 green,PR mergeable。
P50/P99 如后续统一 performance evidence 流程需要可继续记录,但按前一轮 review 结论不作为本 PR 的代码 blocker。当前实现与 #550 最终架构目标一致。
Summary
Implements #550 against
dev, following the latest owner architecture addenda.SharpLinkCompressionOptionscapability-only (Providers/ wire profiles); initial Request/Response send policies live independently in Client/Server build composition.CompressionPolicyControlcapability.(ResponseCompressionPreferenceGeneration, AllowResponseCompression).ResponseCompressionPreferenceUpdate/ResponseCompressionPreferenceAckcontrol frames for later changes.AppliedGeneration >= requested) with per-session O(1) in-flight/latest coalescing.SetResponseCompressionPreferenceAsync; propagation is non-fail-fast across captured sessions, closed sessions leave deterministically, and failures are reported only after the captured cohort has been attempted.Targeted acceptance evidence
Enabled = falsewith providers still negotiated, followed by runtime enable on the same live connection.Setcall.RpcSessionsend queue: an earlierResourceExhausteddoes not prevent a later healthy session from receiving and ACKing the update.OperationCanceledException, and never turns into anAggregateException.SharpLinkCompressionOptions.Minimum*runtime authority remains.Validation
Validated as one clean implementation commit on the current
devlineage (39f980c268632f7469bf2a9c5601e212c9a02c9eonc324ae237f81d45f8df2e0734c312d91ba002fed) using an isolated GitHub Actions final gate:devstability check before publicationDynamic compression hot-path evidence
Same unary byte[] RPC shape over loopback TCP, median of five measured rounds after warmup:
The disabled negotiated path showed no measured allocation increase versus the no-provider path, while the semantic attempt counters prove that only the enabled state reaches the compression provider.
Pull-request CI
Current-head PR checks are green:
The first review pass is fully resolved. The second review follow-up fixes caller-cancellation propagation and completes the compression runtime documentation; the updated head has passed the full isolated gate and current-head PR CI.
Closes #550