Conversation
|
🚅 Deployed to the rivet-pr-4181 environment in rivet-frontend
|
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. |
PR Review: fix: docker cache issues, dashboardsSummaryThis PR fixes two bugs (a potential divide-by-zero in the workflow engine and a missing cache-invalidation trigger in the build system), plus dashboard improvements for Grafana. The changes are small and well-scoped. engine/packages/gasoline/src/db/kv/mod.rs — Bug FixThe core fix (line 1177): // Before
let active_worker_count = active_worker_ids.len() as u64;
// After
let active_worker_count = active_worker_ids.len().max(1) as u64;This prevents a divide-by-zero / modulo-by-zero panic when
Both would panic with a zero denominator in Rust's debug builds (or return 0 via wrapping in release with One concern worth noting: when Suggestion: Consider adding a brief comment before the // Prevent division by zero; if no active workers are found, treat count as 1
// so all workflows are assigned to the current worker.
let active_worker_count = active_worker_ids.len().max(1) as u64;engine/packages/util/build.rs — Cache Fixprintln\!("cargo:rerun-if-env-changed=OVERRIDE_GIT_SHA");Correct fix. Without this instruction, Cargo would not re-run the build script when the engine/docker/universal/Dockerfile — Debug Echoecho "Copying binary" && \
cp target/$CARGO_BUILD_MODE/rivet-engine /app/dist/This is a minor debugging aid added during the cache-investigation work. It's harmless but adds noise to CI/build logs permanently. Consider removing it once the cache issue is confirmed resolved, or keeping it only if it provides useful build-time progress visibility. Grafana Dashboardsgasoline.json:
pegboard.json:
Summary
The Rust fixes are solid. The main actionable issue is the mismatched |
| "legendFormat": "{{worker_id}}", | ||
| "expr": "sum by (sub_workflow_name) (rate(rivet_gasoline_workflow_dispatched_total{rivet_project=~\"$project\",rivet_datacenter=~\"$datacenter\",workflow_name=~\"$workflow_name\"} [$__rate_interval]))", | ||
| "format": "heatmap", | ||
| "legendFormat": "{{signal_name}}", |
There was a problem hiding this comment.
Legend format references {{signal_name}} but the query groups by sub_workflow_name. This mismatch will result in empty or incorrect legend labels.
Fix:
"legendFormat": "{{sub_workflow_name}}",| "legendFormat": "{{signal_name}}", | |
| "legendFormat": "{{sub_workflow_name}}", |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| "editorMode": "code", | ||
| "expr": "sum(increase(rivet_gasoline_load_shedding_ratio_bucket{rivet_project=~\"$project\",rivet_datacenter=~\"$datacenter\"} [$__rate_interval])) by (le)", | ||
| "expr": "sum by (sub_workflow_name) (rate(rivet_gasoline_workflow_dispatched_total{rivet_project=~\"$project\",rivet_datacenter=~\"$datacenter\",workflow_name=\"\"} [$__rate_interval]))", | ||
| "format": "heatmap", |
There was a problem hiding this comment.
Format is set to "heatmap" but the panel type is "timeseries" (line 1724). This format mismatch will cause visualization errors.
Fix:
"format": "time_series",Or remove the format field entirely to use the default for timeseries panels.
| "format": "heatmap", | |
| "format": "time_series", |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
3dbb6ec to
69d4538
Compare
Merge activity
|
# Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes

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: