[HWORKS-2895] Correct the tag-history guide against what ships - #657
Open
jimdowling wants to merge 1 commit into
Open
jimdowling wants to merge 1 commit into
jimdowling wants to merge 1 commit into
Conversation
https://hopsworks.atlassian.net/browse/HWORKS-2895 Correct the tag-history guide against what ships. Four things in it were either wrong or had gone stale since logicalclocks#637 merged. The interval query ordered CLOSED before OPENED at an equal event_time. That is backwards for an attach and a detach that share a millisecond: it puts the CLOSED ahead of the OPENED it followed, LEAD leaves that OPENED with no removed_at, and a removed tag reads as current from then on. The dashboards in okr-dashboards order by (event_time, id) instead, because id is the insertion order and the writer emits a change as CLOSED then OPENED in one transaction. The guide now matches them, and states the residual: RonDB allocates id per SQL node, so two events written a millisecond apart through different nodes can still order arbitrarily. The baseline paragraph said an existing attachment is timed from when it was attached without saying what that means for a reader. That start predates recording, so the first interval of a pre-existing attachment covers time nobody observed and raises the average time-in-state of any report including it. It is a lower bound, and now says so. The note titled "Records intent, no behaviour yet" said nothing reads the flag. Something does now, so it pointed users at the opposite of what will ship. The creation section also named a checkbox, Archive deleted tags, that the UI does not have; the label is Archive tag history. Finally, a new subsection documents the upgrade step. Releases before 5.2 accepted archive at schema creation and stored it with no reader, so a schema created with it on has the flag set and no history, and an upgrade starts one only when the flag is set. Re-issuing PUT /tags/{name}/archive seeds the baseline, and is safe to repeat: the backfill covers only attachments that have no history yet. Signed-off-by: Jim Dowling <jim@logicalclocks.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Follow-up to #637, which merged before these were caught. Four corrections to the tag-history guide, all of them places where the page says something the shipped code does not do.
1. The interval query's tie-break was wrong. The page ordered
CLOSEDbeforeOPENEDat an equalevent_time. For an attach and a detach that share a millisecond that is backwards: it puts theCLOSEDahead of theOPENEDit followed,LEADleaves thatOPENEDwith noremoved_at, and a tag that was removed reads as current from then on. The dashboards inokr-dashboardsorder by(event_time, id)instead, becauseidis the insertion order and the writer emits a change asCLOSEDthenOPENEDinside one transaction. Demonstrated on a cluster: same-millisecond attach and detach gives theOPENEDaremoved_atunder the new ordering andNULLunder the old one. The page now matches the dashboards and states the residual, which is that RonDB allocatesidper SQL node, so two events a millisecond apart through different nodes can still order arbitrarily.2. The baseline paragraph did not say what the timestamp means. An attachment that already existed is timed from when it was attached, which predates recording, so its first interval covers time nobody observed and raises the average time-in-state of any report including it. It is a lower bound, and the page now says so instead of leaving a reader to average over it.
3. A note titled "Records intent, no behaviour yet" said nothing reads the flag. Something does now, so the page told users the opposite of what will ship. The creation section also named a checkbox,
Archive deleted tags, that the UI does not have; the label isArchive tag history(SchematisedTagsCreateForm.tsx:281).4. New: the upgrade step. Releases before 5.2 accepted
archiveat schema creation and stored it with no reader, so a schema created with it on has the flag set and no history. An upgrade does not start one, because the baseline is written when the flag is set. Re-issuingPUT /hopsworks-api/api/tags/{name}/archive?value=trueseeds it, and is safe to repeat:TagHistoryFacade.enableArchivebackfills only attachments that have no history yet.On point 4, one correction to what the review of ee#3110 assumed.
branch-5.1-eecarries V93'sarchivecolumn and accepts the flag at schema creation, but it has noPUT {name}/archivetoggle:setArchiveis absent fromTagSchemasControllerIfacethere, and that interface's own javadoc says of the flag "Nothing reads it". So the gap is reached through schema creation rather than through a toggle, which is why the step is documented against schema creation.Testing
Documentation only; no code in this PR, so there are no loadtests to run. The gates that do apply pass locally with CI's own commands:
hopsworks-docs markdownlint(243 files, 0 errors) andhopsworks-docs snakeoil(all checks passed, tree left clean, which is what CI'sgit diff --exit-codeenforces).🤖 Generated with Claude Code