Change #1293836 had a related patch set uploaded (by Xcollazo; author: Xcollazo):

[analytics/refinery/source@master] Drop _within_90_days revert tier; full-table revert seed; scoped back-patch

https://gerrit.wikimedia.org/r/1293836

Spike complete — full-table revert detection is viable.

Performance (May 2026 production data, 6 days)

All six runs cleared the ≤ 1 hour acceptance threshold. The outliers (May 1: International Workers' Day, May 4: Star Wars day) reflect high edit volume on the source side, not the seed scan. Typical days cluster around 25–31 min. The table at test time was ~8.1B rows / ~540 GB (April 2026 snapshot seed).

What changed

• revert_seed CTE: 90-day lower bound removed; now a full-table scan. Incoming revision_ids are excluded via LEFT ANTI JOIN to prevent double-counting on reruns or when admin ops re-emit events for already-committed revisions (would have caused seconds_to_revert=0 self-revert false positives).
• revert_annotations: the four _within_90_days bounded-tier columns dropped entirely. The authoritative tier now has no time bound — TRUE when detected, NULL otherwise.
• MERGE 2 (back-patch): retained but redesigned. Instead of a 90-day time bound on the seed, the seed is scoped to revision_text_sha1 IN (today's incoming sha1s). This avoids a second full-table scan while still catching cross-day reverts of arbitrarily old rows. Same ANTI JOIN guard applied.
• MWHistorySnapshotMerger: revert_seconds CTE and its LEFT JOIN removed (were only needed to derive the bounded tier). All four _within_90_days columns removed from projection, UPDATE, and INSERT.
• SnapshotMerger DELETE guard: AND t.event_entity = 'revision' added to the source='events' DELETE clause. Without this, page and user event rows would be deleted when the monthly merger runs, since projected_monthly only produces revision rows and every page/user row is unconditionally "not matched by source". T427328 tracks the full fix (extending the merger to also reconcile page and user entities).

Code

Gerrit: https://gerrit.wikimedia.org/r/c/1293836

Update — algorithm correctness + perf

Earlier spike claim of "semantically equivalent to wmf.mediawiki_history monthly" was wrong. The sha1_rank+1 self-join missed the canonical A → B → A revert pattern (intermediate different-sha rev inside a base→reverter window — monthly's wider-revert branch catches it; daily didn't), over-marked the sha-series base as reverted (monthly leaves it — the base's content survives), and over-marked same-sha rank≥2 reverters. The daily test suite had been hiding the divergence by only testing same-sha sequences with the intermediate omitted.

Fix: rewrote revert detection in MWHistoryDeltaWriter to implement the wider-revert algorithm directly in SQL — for each revision R, find the MIN-by-(ts, rev_id) reverter whose base was processed strictly before R and which itself comes after R. Same logic in the back-patch (MERGE 2). Seed scope broadened from sha1-IN to page_id-IN (wider reverts can cross sha-series). Ported the monthly's 9-revision "nested reverts" golden test from TestDenormalizedRevisionsBuilder as the integration check; all 78 unit tests pass.

Perf: hot Wikidata pages (top page ~2.1M revisions) made the join fan out to ~10¹² tuples on a single Spark task — MERGE 1 took 1.4h and failed, MERGE 2 spilled 70 GiB. Fixed with single-column filters pushed pre-shuffle: MERGE 1 restricted to incoming-on-both-sides; MERGE 2 to seed-rows-not-yet-annotated × incoming reverters (reads revision_is_identity_reverted from the target). First day after the fix: MERGE 1 ~15 min, MERGE 2 awaiting next run.

Caveats (documented in code): both filters assume in-order daily runs. Out-of-order backfills or late-arriving events (rev_dt earlier than delivery day) can leave a bounded set of annotations missed in the in-month window; the next MWHistorySnapshotMerger run heals at month-close. A time-bounded alternative is sketched in the candidates CTEs but kept unimplemented — the cross-side range predicate isn't pushable in OSS Spark's SortMergeJoin, so it wouldn't address the perf cliff.

Gerrit: https://gerrit.wikimedia.org/r/c/analytics/refinery/source/+/1293836 (branch T427314-full-table-revert-detection). Continuing verification against the May 2026 monthly snapshot once it lands.

Update — two algorithm bug fixes, attempted MERGE unification reverted

After the wider-revert spike ran clean on May 1-7 (mean ~30 min/day, max ~55 min, late-event gap 50 rows in 12.3M ≈ 4×10⁻⁶), Joseph flagged two correctness bugs in the monthly's reference algorithm that we should not inherit:

1. Same-parent-id rule. A reverter sharing revision_parent_id with the row it would revert is a page-merge artifact, not a real revert — page merges leave two revisions with identical content pointing at the same pre-merge ancestor. The new daily skips these via r.reverter_parent_id IS DISTINCT FROM rev.revision_parent_id in the candidates CTE (NULL-safe — two unrelated page-creates that happen to share a sha1 are also skipped).

2. NULL revision_text_sha1 filter. The monthly currently includes NULL-sha1 rows in its (page, sha) grouping, and Spark treats NULL=NULL in PARTITION BY, so all suppressed-text revisions on a page share one fake "sha-series" and rank against each other. The daily now excludes them in both revert_seed and revert_candidates. Tracked upstream as a separate fix; the daily intentionally diverges in the meantime.

Both fixes added to MERGE 1 and MERGE 2 along with positive sanity tests (parent-id pair → no revert; NULL-sha pair → no revert). 80 tests pass.

Attempted: unified MERGE 1 + MERGE 2 into one Iceberg MERGE INTO. Looked clean — multiple WHEN MATCHED clauses dispatched on an op tag, one shared sha1_ranked / first_reverter pipeline, one shuffle. Reverted: the unified source's revision_ids span the full revision_id range (back-patches target arbitrary historical rows), which defeats Iceberg COW's target file pruning. Symptom on first run: the Exchange in front of ReplaceData carried the full 8B-row target. Two separate MERGEs each have tightly-scoped source keys (today's incoming → current month; back-patches → a few thousand files) and prune correctly. The candidates / first_reverter computation runs twice per day — fine; the redundancy is the cost of partition pruning. Documented as a code-level comment and as a reusable Iceberg COW pattern note.

Considered: lag() / window-only revert detection. Tempting because the small reverters set (~1.8 MB/day) is broadcastable and the per-page chronological iteration matches Window partitioning. Discarded: lag() only sees the immediately-prior same-sha row, not the wider-revert "MIN-by-(ts, id) pending reverter across all base→reverter windows" that the monthly computes. The closest pure-window approximation (collect_list over an UNBOUNDED PRECEDING window) materializes the full page history per row — exactly the hot-page blow-up we're trying to avoid. Kept the current min_by-over-candidates structure with a BROADCAST hint on the small reverters set.

Perf addition (now testing). Added /*+ BROADCAST(r) */ on the candidates join. Reverters set is ~23k rows on the busiest day (~1.8 MB), trivially under autoBroadcastJoinThreshold. Eliminates the SortMergeJoin shuffle on the rev side. Currently running on a fresh ..._bugfix table to (a) verify the bug fixes produce only the expected diff vs _wider_revert and (b) measure the broadcast win.

I have not looked at the code, but this definition is incorrect:

Same-parent-id rule. A reverter sharing revision_parent_id with the row it would revert is a page-merge artifact, not a real revert

The definition should be:
A Reverting revision R shouldn't considered a revert if it has its revisionParentId being the revisionId of the revert it reverts-to.

Bug-fix sanity checks — both pass

(1) NULL-sha rows never carry a revert annotation. Spark treats NULL = NULL in PARTITION BY, so the algorithm has to filter NULL sha1 explicitly.

SELECT
    sum(CASE WHEN revision_text_sha1 IS NULL
              AND revision_is_identity_revert   = true THEN 1 ELSE 0 END) AS null_sha_marked_is_revert,
    sum(CASE WHEN revision_text_sha1 IS NULL
              AND revision_is_identity_reverted = true THEN 1 ELSE 0 END) AS null_sha_marked_reverted
FROM xcollazo.mediawiki_history_incremental_v1_aligned
WHERE source = 'events' AND event_entity = 'revision';

(2) No-op rule: a row flagged is_revert=TRUE must NOT have its revision_parent_id pointing at an earlier same-sha occurrence on the same page (i.e. it's not a no-op duplicate of its parent).

SELECT count(*) AS noop_marked_as_revert_violations
FROM xcollazo.mediawiki_history_incremental_v1_aligned r
WHERE r.source       = 'events'
  AND r.event_entity = 'revision'
  AND r.revision_is_identity_revert = true
  AND r.revision_parent_id IS NOT NULL
  AND EXISTS (
    SELECT 1
    FROM xcollazo.mediawiki_history_incremental_v1_aligned prev
    WHERE prev.wiki_id            = r.wiki_id
      AND prev.page_id            = r.page_id
      AND prev.revision_text_sha1 = r.revision_text_sha1
      AND prev.revision_id        = r.revision_parent_id
  );

Late-event distribution and gap estimate

WITH t AS (
  SELECT
    event_timestamp                                                   AS rev_ts,
    to_timestamp(control_map['revision_update_dt'])                   AS write_ts,
    revision_is_identity_reverted, revision_is_identity_revert
  FROM xcollazo.mediawiki_history_incremental_v1_aligned
  WHERE source = 'events' AND event_entity = 'revision'
)
SELECT
  CASE
    WHEN unix_timestamp(write_ts) - unix_timestamp(rev_ts) <  3600    THEN '01 <1h (normal)'
    WHEN unix_timestamp(write_ts) - unix_timestamp(rev_ts) <  86400   THEN '02 1h-1d'
    WHEN unix_timestamp(write_ts) - unix_timestamp(rev_ts) <  604800  THEN '03 1d-7d'
    WHEN unix_timestamp(write_ts) - unix_timestamp(rev_ts) <  2592000 THEN '04 7d-30d'
    ELSE '05 >30d'
  END                                                                  AS lateness_bucket,
  count(*)                                                             AS n,
  sum(CASE WHEN revision_is_identity_reverted = true THEN 1 ELSE 0 END) AS marked_reverted
FROM t GROUP BY 1 ORDER BY 1;

99.996% of events arrive within an hour of rev_ts. The late tail is dominated by page-import patterns (rev_dt from the original revision, not delivery time).

Upper-bound gap heuristic — NULL-annotated late rows that have a same-sha occurrence later on the same page (i.e. rows where the daily writer *might* have missed a reverter that arrived in a later day):

SELECT count(*) AS suspect_late_unannotated
FROM xcollazo.mediawiki_history_incremental_v1_aligned late
WHERE late.source = 'events'
  AND late.event_entity = 'revision'
  AND late.revision_is_identity_reverted IS NULL
  AND late.event_timestamp < to_timestamp(late.control_map['revision_update_dt']) - INTERVAL 1 DAYS
  AND EXISTS (
    SELECT 1 FROM xcollazo.mediawiki_history_incremental_v1_aligned later
    WHERE later.wiki_id = late.wiki_id AND later.page_id = late.page_id
      AND later.revision_text_sha1 = late.revision_text_sha1
      AND later.event_timestamp > late.event_timestamp
      AND later.revision_id     != late.revision_id
  );

50 / 12.3M = 4×10⁻⁶, matching the commit-message claim. And the sample is again heavily dominated by the same dewiki phoiac9... blanking chain (≈50% of the sampled 30 rows are that page), meaning the heuristic *over*-counts — most of the NULLs it flags are the algorithm correctly excluding no-op duplicates, not a gap. So the real late-event gap is ≤ 50 rows.

Net

• Both correctness sanity checks pass (zero violations).
• 639 inner-chain catches and 50 no-op excludes vs the prior structure, both explainable.
• 99.996% of events on-time; late-event gap is bounded above by 50 rows.
• Authoritative diff vs wmf.mediawiki_history is the remaining check, blocked on the May 2026 monthly snapshot landing.

Implementation notes — hot-page join and pipeline invariant

The revert join is on (wiki_id, page_id) only, which means a single hot page (Wikidata bot-target items, Commons sandboxes — up to ~2.1M revisions per page empirically) can land all its revisions on one Spark task and fan out to O(N×R) tuples. Two layers bound this:

• reverters_with_base is restricted to is_incoming = TRUE AND is_real_reverter = TRUE — roughly ~1.8 MB/day at WMF scale (~23k rows on the busiest May 2026 day). With the /*+ BROADCAST(r) */ hint this set is shipped to every probe task, eliminating the shuffle on the revision side entirely.
• Each candidates_m1/m2 CTE applies its own rev-side filter (incoming-only for M1, seed-unannotated-only for M2), so the probe side is also tightly bounded per MERGE.

A unified candidates CTE (union of both sides) was tried and reverted: each MERGE re-ran the chain on the full union, doubling upstream work and raising mean daily run time from ~30 min to ~68 min over the 7-day May 2026 sample.

Correctness assumption: both candidates CTEs assume in-order daily runs (seed.max_ts < incoming.min_ts). Out-of-order backfills or late-arriving events (rev_dt before delivery day) leave a bounded set of annotations undetected in the in-month window. MWHistorySnapshotMerger heals these at monthly close.

Change #1293836 merged by jenkins-bot:

[analytics/refinery/source@master] Daily revert detection: align with monthly DenormalizedRevisionsBuilder

https://gerrit.wikimedia.org/r/1293836