direct: detect stale dashboard publishes via publish/update timestamps - #6119
Open
denik wants to merge 2 commits into
Open
direct: detect stale dashboard publishes via publish/update timestamps#6119denik wants to merge 2 commits into
denik wants to merge 2 commits into
Conversation
DoRead computed a dashboard's published state as "does GetPublished succeed", but once a dashboard is published GetPublished keeps returning the previous revision even after the draft is updated, so that check cannot tell whether the published content is current. Compare the published revision_create_time against the draft update_time instead: published is true only when GetPublished succeeds AND revision_create_time >= update_time. When the draft was updated without a republish, published becomes false, which differs from the desired true and the planner schedules a republish. Verified on AWS, GCP, Azure and dogfood: a publish always leaves revision_create_time >= update_time, and a draft update pushes update_time ahead until the next publish (so there is no perpetual-republish risk). The draft etag, by contrast, does not reliably change on a display-name update on cloud, so it cannot be used for this. The testserver now emits strictly-increasing RFC3339Nano timestamps (nextTimestamp) so update/publish ordering is deterministic. published is also excluded from config-remote-sync as an internal state field. Co-authored-by: Isaac
Contributor
Approval status: pending
|
Collaborator
Integration test reportCommit: 4334fe9
8 interesting tests: 4 SKIP, 3 KNOWN, 1 RECOVERED
Top 7 slowest tests (at least 2 minutes):
|
… update
Deploys a dashboard, makes an out-of-band draft update without republishing, and
asserts `bundle plan` reports the dashboard's published state needs an update
(published: {old: true, remote: false}). Before the timestamp-based fix, DoRead
reported published=true (GetPublished still returns the previous revision), so
the stale publish was missed and the plan showed no change.
Direct engine only: published is a direct-engine state field.
Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Detect stale dashboard publishes in the direct engine by comparing the published
revision_create_timeagainst the draftupdate_time, instead of treating "apublished version exists" as "the published content is current".
A dashboard is deployed as a draft update followed by a separate publish. Once
published,
GetPublishedkeeps returning the previous revision even after thedraft changes, so the old
published = (GetPublished succeeds)check could nottell that the published content was stale. Now
publishedis true only whenrevision_create_time >= update_time; a draft updated without a republish readsas
published=falseand the planner surfaces a pending republish.Why not the draft etag
Verified on AWS, GCP and Azure: the draft etag tracks the serialized dashboard
content, so a metadata-only draft change (e.g.
display_name) with an unchangedserialized payload does not bump the etag — while
update_timestill advances.So an etag-based "did the draft change since we published" check would miss that
case; the
revision_create_timevsupdate_timecomparison does not. A publishalways leaves
revision_create_time >= update_time, so there is noperpetual-republish risk.
Scope
This improves detection only. Recovering a stale publish via a plain re-deploy
also requires persisting state before the publish call (so the etag-drift
"modified remotely" guard does not block); that is a separate change. Here, a
stale publish is detected in the plan but recovery may still require
--force(unchanged from today).
Tests
New regression test
republish-after-draft-update: an out-of-band draft updatemakes
bundle planreportpublishedneeds an update (it did not before). Goldensupdated for
detect-changeandpublish-failure-stale-content. The testserver nowemits strictly-increasing timestamps so update/publish ordering is deterministic.