Skip to content

feat(eap-items): add version column to eap_items_1_downsample_64_local - #8426

Open
onewland wants to merge 1 commit into
masterfrom
feat/eap-items-version-downsample-64
Open

feat(eap-items): add version column to eap_items_1_downsample_64_local#8426
onewland wants to merge 1 commit into
masterfrom
feat/eap-items-version-downsample-64

Conversation

@onewland

@onewland onewland commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Stack

  1. feat(eap-items): add version column to eap_items_1 #8424eap_items_1 (merged)
  2. feat(eap-items): add version column to eap_items_1_downsample_8_local #8428eap_items_1_downsample_8_local
  3. feat(eap-items): add version column to eap_items_1_downsample_64_local #8426eap_items_1_downsample_64_local
  4. feat(eap-items): add version column to eap_items_1_downsample_512_local #8427eap_items_1_downsample_512_local
  5. feat(eap-items): carry version through the downsample materialized views #8430 — downsample MVs carry version

Merge in order; each targets the one above it.


Adds the version column to the downsample_64 tier.

ALTER TABLE eap_items_1_downsample_64_local ADD COLUMN IF NOT EXISTS version UInt64 DEFAULT 0 CODEC (ZSTD(1));

Why

Part of getting the eap_items schemas aligned with the target ReplacingMergeTree (keyed on version) so that existing parts can be attached onto the new tables. This mirrors the column added to eap_items_1 in migration 0064.

Schema-only — no engine change, so no dedup behaviour changes yet.

Why DEFAULT 0

ADD COLUMN does not rewrite existing parts, so rows written before this migration have no version on disk and the default is evaluated at read/merge time. That makes a non-deterministic default actively dangerous here: under ReplacingMergeTree(version) a now64() default gives legacy rows a version of "whenever the merge ran", which always beats a genuine write. Verified on CH 25.3 — a real update was silently discarded in favour of the stale row against a DEFAULT now64() destination, and won as expected against a DEFAULT 0 one.

0 is the sentinel for "written before versioning existed" and deterministically loses to every real write, matching the received_at precedent in snuba/manual_jobs/create_eap_received_at_version_test.py.

Why local only

Unlike 0064, this does not touch eap_items_1_downsample_64_dist (or _dist_ro):

  • We never query version — it is internal bookkeeping for the cutover and is absent from the storage YAML, so the query layer cannot generate SQL naming it.
  • The downsample tiers are fed by materialized views that write straight to *_local (swap_downsample_materialized_views uses destination_table_name={prefix}_local, target=OperationTarget.LOCAL), so the distributed table is never an insert target.

A Distributed table declaring a subset of its local table's columns behaves normally — verified that reads and SELECT * through the narrower dist table work, and that an insert routed through it still lets the local default fire. Only an explicit SELECT version through the dist table errors, and nothing does that.

Follow-up

The downsample MVs do not yet carry version through from the source, so rows they insert will read 0 rather than the source row's version. Fixing that needs an MV rebuild and is deliberately out of scope here.

Testing

  • tests/migrations/test_runner.py::test_no_schema_differences
  • tests/migrations/test_runner.py::test_run_and_reverse_all

@onewland
onewland requested review from a team as code owners September 3, 2026 00:08
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

This PR has a migration; here is the generated SQL for ./snuba/migrations/groups.py ()

-- start migrations

-- forward migration events_analytics_platform : 0066_add_version_column_downsample_64
Local op: ALTER TABLE eap_items_1_downsample_64_local ON CLUSTER 'cluster_one_sh' ADD COLUMN IF NOT EXISTS version UInt64 DEFAULT 0 CODEC (ZSTD(1));
-- end forward migration events_analytics_platform : 0066_add_version_column_downsample_64




-- backward migration events_analytics_platform : 0066_add_version_column_downsample_64
Local op: ALTER TABLE eap_items_1_downsample_64_local ON CLUSTER 'cluster_one_sh' DROP COLUMN IF EXISTS version;
-- end backward migration events_analytics_platform : 0066_add_version_column_downsample_64

Base automatically changed from feat/eap-items-version-downsample-8 to master September 3, 2026 20:03
onewland added a commit that referenced this pull request Sep 3, 2026
…#8428)

### Stack

1. ~#8424 — `eap_items_1` (merged)~
2. #8428 — `eap_items_1_downsample_8_local`
3. #8426 — `eap_items_1_downsample_64_local`
4. #8427 — `eap_items_1_downsample_512_local`
5. #8430 — downsample MVs carry `version`

Merge in order; each targets the one above it.

---

Adds the `version` column to the `downsample_8` tier.

```sql
ALTER TABLE eap_items_1_downsample_8_local ADD COLUMN IF NOT EXISTS version UInt64 DEFAULT 0 CODEC (ZSTD(1));
```

### Why

Part of getting the `eap_items` schemas aligned with the target
`ReplacingMergeTree` (keyed on `version`) so that existing parts can be
attached onto the new tables. This mirrors the column added to
`eap_items_1` in migration 0064.

Schema-only — no engine change, so no dedup behaviour changes yet.

### Why `DEFAULT 0`

`ADD COLUMN` does not rewrite existing parts, so rows written before
this migration have no `version` on disk and the default is evaluated at
read/merge time. That makes a non-deterministic default actively
dangerous here: under `ReplacingMergeTree(version)` a `now64()` default
gives legacy rows a version of "whenever the merge ran", which always
beats a genuine write. Verified on CH 25.3 — a real update was silently
discarded in favour of the stale row against a `DEFAULT now64()`
destination, and won as expected against a `DEFAULT 0` one.

`0` is the sentinel for "written before versioning existed" and
deterministically loses to every real write, matching the `received_at`
precedent in `snuba/manual_jobs/create_eap_received_at_version_test.py`.

### Why local only

Unlike 0064, this does not touch `eap_items_1_downsample_8_dist` (or
`_dist_ro`):

- We never query `version` — it is internal bookkeeping for the cutover
and is absent from the storage YAML, so the query layer cannot generate
SQL naming it.
- The downsample tiers are fed by materialized views that write straight
to `*_local` (`swap_downsample_materialized_views` uses
`destination_table_name={prefix}_local`,
`target=OperationTarget.LOCAL`), so the distributed table is never an
insert target.

A Distributed table declaring a subset of its local table's columns
behaves normally — verified that reads and `SELECT *` through the
narrower dist table work, and that an insert routed through it still
lets the local default fire. Only an explicit `SELECT version` through
the dist table errors, and nothing does that.

### Follow-up

The downsample MVs do not yet carry `version` through from the source,
so rows they insert will read `0` rather than the source row's version.
Fixing that needs an MV rebuild and is deliberately out of scope here.

### Testing

- `tests/migrations/test_runner.py::test_no_schema_differences`
- `tests/migrations/test_runner.py::test_run_and_reverse_all`
Mirrors the version column added to eap_items_1 in 0064, aligning the
downsample_64 tier's schema with its target ReplacingMergeTree ahead of
attaching existing parts.

Only the local table is touched. We never query `version`, and the
downsample tiers are fed by materialized views that write straight to
*_local, so the distributed table is neither a read nor an insert path
for this column.

Defaults to 0, the sentinel for rows written before versioning existed,
which deterministically loses to every real write.
@onewland
onewland force-pushed the feat/eap-items-version-downsample-64 branch from 369eaff to 6dd9519 Compare September 3, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants