fix(pegboard): add threshold parameters to runner protocol metadata#4247
Conversation
|
🚅 Deployed to the rivet-pr-4247 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
test comment - please ignore |
|
simple test 2 |
|
test with gt sign and lt sign |
PR Review: fix(pegboard): add threshold parameters to runner protocol metadataSummaryThis PR bumps the runner protocol to v6 to add two new fields to Bugs / Correctness1. Mutation of shared const definition = lookupInRegistry(this.#config, actorConfig.name);
// ...
definition.config.options.onSleepTimeout = Math.min(...);
definition.config.options.onDestroyTimeout = Math.min(...);If 2. Truthiness check on if (protocolMetadata.serverlessDrainGracePeriod) {The type is 3. In this.#protocolMetadata.runnerLostThreshold > 0
4. serverless_drain_grace_period: is_serverless
.then(|| pb.serverless_drain_grace_period() as i64),A very large Code Quality5. metadata: v6::ProtocolMetadata {
runner_lost_threshold: init.metadata.runner_lost_threshold,
actor_stop_threshold: 0,
serverless_drain_grace_period: None,
},Hardcoding 0 means a v5 runner will see 6. Missing V5 variant in pub enum ToServerMk2 {
V4(v4::ToServer),
V6(v6::ToServer), // no V5
}This is well-handled by the comment and the Performance
Minor Nits
Test CoverageNo tests are added for:
Given the crash/timeout implications of getting these thresholds wrong, at minimum a unit test for the actor-driver clamping logic would be valuable. |
3008d9c to
13b1948
Compare
13b1948 to
76e52b6
Compare
76e52b6 to
ce7323c
Compare
121bf70 to
3da1f9e
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: