shreds: flag lagging publishers on the shred publishers page - #666
shreds: flag lagging publishers on the shred publishers page#666SinaVafadar wants to merge 4 commits into
Conversation
Pulls validators reported as not Healthy from dzf_data.lagged_validators and annotates matching shred publishers with a new Lagging column. Adds dzf_data database plumbing (config, env var, API wiring) and a non-fatal lookup in FetchPublisherCheckData so the page still renders if the table is unavailable.
|
🔗 Preview: https://pr-666.data.malbeclabs.com |
ben-dz
left a comment
ben-dz
left a comment
There was a problem hiding this comment.
The lagging column ships with an internal contradiction and no verification of its success path. The backend flags any status != 'Healthy' (and the sort uses that boolean), but the icon is red only for the exact string "Action Needed" — a value that appears nowhere in the backend and isn't documented anywhere — so any other non-Healthy status sorts as lagging while displaying a green check. The test harness never sets DZFDataDB, so fetchLaggedValidators always errors in tests and every existing test exercises only the warn-and-degrade branch; the read-only prod ClickHouse user has no grant on dzf_data.lagged_validators, so the deployed API user's grant should be confirmed before merge or the column ships permanently green. Also worth addressing: the query assumes the externally-written table holds only current state (no recency bound or dedup), and lagging_status is never actually shown in the UI despite the PR description saying it appears in the cell and on hover.
| </td> | ||
| <td className="px-4 py-3 text-center"> | ||
| <StatusIcon | ||
| ok={pub.lagging_status !== "Action Needed"} |
There was a problem hiding this comment.
This predicate disagrees with the backend and the sort. The backend sets lagging=true for any status != 'Healthy' (publisher_check.go:386) and the sort comparator uses that boolean (line 417), but the icon is red only for the exact string "Action Needed" — a value that appears nowhere in the backend or the repo, from a table whose status vocabulary is undocumented. Any other non-Healthy value (or a casing change by the external writer) yields a row that sorts as lagging while showing a green check; rows with status='' are also flagged lagging with a green icon, and an undefined lagging_status during deploy skew renders green too. Use one predicate: ok={!pub.lagging} (also handles undefined), or filter the SQL to status = 'Action Needed' so the flag, sort, and icon agree.
| // fetchLaggedValidators returns a map of validator vote pubkey to its lagging | ||
| // status for every validator currently reported as not Healthy in | ||
| // dzf_data.lagged_validators. | ||
| func (a *API) fetchLaggedValidators(ctx context.Context) (map[string]string, error) { |
There was a problem hiding this comment.
The success path of this function is never verified anywhere. The test harness (api/testing/api.go) never sets DZFDataDB, so in every existing test this builds FROM ` `.lagged_validators with an empty identifier and always errors — the tests cited in the PR only exercise the warn-and-degrade branch. Fix: set DZFDataDB in the harness constructors (e.g. to the per-test dbName), seed a lagged_validators fixture following the createPublisherShredStatsTable pattern with a Healthy row, a non-Healthy row matched to a fixture vote pubkey, and an unmatched pubkey, and assert lagging/lagging_status on the response — this also pins the status-string contract. Separately, the read-only prod ClickHouse user has no SELECT grant on dzf_data.lagged_validators (verified), so please confirm the deployed API user's grant before merge; if it's missing, the column ships permanently green with only a server-side warn every 30s.
| query := fmt.Sprintf(` | ||
| SELECT validator_vote_public_key, status | ||
| FROM `+"`%s`"+`.lagged_validators | ||
| WHERE status != 'Healthy' AND validator_vote_public_key != ''`, a.DZFDataDB) |
There was a problem hiding this comment.
This assumes the table holds only current state: no recency bound, no FINAL/argMax dedup, and arbitrary last-write-wins in the map when a pubkey has multiple rows. The table is written by an external process and its schema/engine exists nowhere in this repo. If it's append-mode (or a pre-merge ReplacingMergeTree), a recovered validator stays flagged; if the writer stops, flags freeze silently. Confirm the write contract (full replace per cycle?) and document it here, or dedup to latest state per pubkey with a recency bound.
| <StatusIcon ok={!pub.publishing_retransmitted} /> | ||
| </td> | ||
| <td className="px-4 py-3 text-center"> | ||
| <StatusIcon |
There was a problem hiding this comment.
lagging_status is never shown: the PR description says the status text appears in the cell and on hover, but StatusIcon accepts only ok and the cell has no title, so the field is shipped to the client and only feeds the string comparison. The legend box above the table also explains the other two icon columns but not "Fast Shreds". Add title={pub.lagging_status || undefined} to the cell and a legend entry (and update the PR description) — or drop the field if it has no UI consumer after unifying the predicate.
| // in dzf_data.lagged_validators. Non-fatal: if the lookup fails we still | ||
| // return publisher data without lagging annotations. | ||
| if lagging, err := a.fetchLaggedValidators(ctx); err != nil { | ||
| slog.Warn("publisher check: lagged validators query failed", "error", err) |
There was a problem hiding this comment.
The page-cache worker refreshes publisher_check every 30s, so any environment where the table is absent (local dev without remote tables) or the grant is missing logs this warning every 30s indefinitely. Consider skipping the lookup when !isMainnet(ctx) and/or probing table availability once at startup.
| } | ||
|
|
||
| // SetDZFDataDB sets the dzf_data database name. | ||
| func SetDZFDataDB(db string) { |
There was a problem hiding this comment.
SetDZFDataDB has no callers — the env-var override in Load() is the only write path. Drop the setter.
|
Removing the Preview label. noticed while debugging something else:
|
2 participants
Summary
/dz/shreds/publishers) that flags validators reported as notHealthyindzf_data.lagged_validators.Healthyvalue from the table) is shown in the cell and on hover, and the column is sortable.dzf_dataClickHouse database (config default,CLICKHOUSE_DZF_DATA_DBenv override, and API plumbing) so the handler can read the table.Details
FetchPublisherCheckDatanow joins publishers againstdzf_data.lagged_validatorsbyvalidator_vote_public_key, selecting rows wherestatus != 'Healthy'. Matching publishers are annotated withlaggingandlagging_status. The lookup is non-fatal — if the table is unavailable the page still renders without lagging annotations. Because the logic lives inFetchPublisherCheckData, the background page cache picks up the new fields automatically.Testing Verification
PublisherCheckhandler tests pass; the lagged-validators lookup degrades gracefully when the table is absent in the test DB (logged warning, publishers returned unannotated).