Skip to content

Commit cc90574

Browse files
thiagohoraclaude
andcommitted
[OPIK-7773] [BE] fix: correct the runbook's restart attribution and the readiness DTO builder
Two more review findings, both in scope. The runbook's self-clearing note had the restart on the wrong path: it read "no extra restart needed on the wrap-first path, and only the already-planned restart on the toggle-first path", which is backwards. Both orderings spend exactly the one planned rolling restart the toggle already requires, and neither needs another; what differs is its position, and with it what closes the mismatch window — the wrap DDL on the toggle-first path (the restart is already done by then), the restart completing on the wrap-first path. Reworded to say that. HealthCheckResponse used a plain @builder, copied from the same record in HealthCheckIntegrationTest, where the convention predates the rule. Records and DTOs take @builder(toBuilder = true) (.agents/skills/opik-backend/SKILL.md:52, which lists bare @builder as the anti-pattern), so the new copy uses it; the older one is left alone as unrelated to this ticket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a810a6b commit cc90574

2 files changed

Lines changed: 5 additions & 3 deletions

File tree

apps/opik-backend/data-migrations/traces-local-v2-cutover/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,10 @@ new table before the EXCHANGE. The replay matches the **full key**, not `id` alo
237237
> point of the check (an instance whose deletes cannot work should not serve), but it changes the cost of the window
238238
> from "delete-path 500s" to "no backend in rotation", so the window must sit **inside the declared maintenance window**
239239
> that `--confirm-maintenance` already asserts for the wrap. It is self-clearing: the probe re-evaluates continuously,
240-
> so rotation returns on the next successful probe after the two sides are back in step — no extra restart needed on
241-
> the wrap-first path, and only the already-planned restart on the toggle-first path.
240+
> so rotation returns on the next successful probe once the two sides are back in step. **Neither ordering needs an
241+
> extra restart** — both spend exactly the one planned rolling restart the toggle already requires; only its position
242+
> differs, and with it what closes the window: on the toggle-first path the restart comes first and the **wrap DDL**
243+
> closes the window, on the wrap-first path the wrap comes first and the **restart completing** closes it.
242244
>
243245
> Prefer **toggle first**, have the `--wrap-only` command ready to run the moment every backend instance is up, and
244246
> keep the window to seconds. Nothing in either direction corrupts data — that is what makes a short window

apps/opik-backend/src/test/java/com/comet/opik/infrastructure/db/healthchecks/ClickHouseTracesTopologyReadinessTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ private void execute(String sql) {
272272
.flatMap(result -> Mono.from(result.getRowsUpdated()))).block();
273273
}
274274

275-
@Builder
275+
@Builder(toBuilder = true)
276276
private record HealthCheckResponse(String name, boolean healthy, boolean critical, String type) {
277277
}
278278
}

0 commit comments

Comments
 (0)