Skip to content

[HWORKS-2895] Correct the tag-history guide against what ships - #657

Open
jimdowling wants to merge 1 commit into
logicalclocks:mainfrom
jimdowling:hopsworks-analytics-tag-history-corrections
Open

jimdowling wants to merge 1 commit into
logicalclocks:mainfrom
jimdowling:hopsworks-analytics-tag-history-corrections

Conversation

@jimdowling

Copy link
Copy Markdown
Contributor

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 CLOSED before OPENED at an equal event_time. For an attach and a detach that share a millisecond that is backwards: it puts the CLOSED ahead of the OPENED it followed, LEAD leaves that OPENED with no removed_at, and a tag that was removed 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 inside one transaction. Demonstrated on a cluster: same-millisecond attach and detach gives the OPENED a removed_at under the new ordering and NULL under the old one. The page now matches the dashboards and states the residual, which is that RonDB allocates id per 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 is Archive tag history (SchematisedTagsCreateForm.tsx:281).

4. New: 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. An upgrade does not start one, because the baseline is written when the flag is set. Re-issuing PUT /hopsworks-api/api/tags/{name}/archive?value=true seeds it, and is safe to repeat: TagHistoryFacade.enableArchive backfills only attachments that have no history yet.

On point 4, one correction to what the review of ee#3110 assumed. branch-5.1-ee carries V93's archive column and accepts the flag at schema creation, but it has no PUT {name}/archive toggle: setArchive is absent from TagSchemasControllerIface there, 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) and hopsworks-docs snakeoil (all checks passed, tree left clean, which is what CI's git diff --exit-code enforces).

🤖 Generated with Claude Code

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant