-
[OPIK-6696] [BE] feat(datasets): copy_from coords on createOrUpdateDatasetItems + applyDatasetItemChanges (#6867)
frostbyte_neo 发布于
2026-05-27 16:30:46 +00:00 - [OPIK-6696] [BE] feat(datasets): snapshot mode on createOrUpdateDatasetItems
Adds an opt-in
snapshot: Booleanfield on DatasetItemBatch. When true, the
new dataset version's row set equals exactly the items in the payload; items
absent from the payload are absent from the new version (deletions implicit).The structural goal is to side-step the INSERT FROM SELECT against the
destination's prior version in applyDelta's copyUnchangedItems step. On
multi-replica ClickHouse that SELECT hits the read-after-write visibility
window described in OPIK-6601, silently dropping unchanged rows and causing
the data loss reported in OPIK-6674 and the qa-v1-stress-* regressions.In snapshot mode the service:
- skips getItemIdsAndHashes against the base version (no classification SELECT)
- skips copyUnchangedItems entirely (no INSERT FROM SELECT on dataset_item_versions)
- runs only insertItems with the payload rows
Same treatment applies to the "version already exists for batch_group_id ->
append" branch of handleGroupedInsertion, which has the same stale-read shape.Default (non-snapshot) behavior is unchanged. Snapshot=true on the very first
version of a dataset (no base) falls through to the existing createFirstVersion
path, which is already INSERT-only.Per-version metadata (tags / change_description / metadata) and per-item
created_at / created_by round-trip are out of scope for this PR per the
ticket's "documented decision" clause; deferred to follow-up tickets if migrate
deep-equal assertions require them.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- test(datasets): assert snapshot-mode 3-version chain preserves per-version row sets
Adds a chain test that walks v1 -> v2 -> v3 entirely in snapshot mode, varying
which items the payload includes at each step (drop, add, re-add). Asserts that:- each version's row set equals exactly its payload (no leakage from prior
versions, no implicit carry-forward) - the data payload of an item carried across two versions round-trips
identically at both points (proves the row content comes from the payload
each time, not from a stale SELECT)
This guards the equivalence claim: under snapshot mode, an N-version replay
produces, per-version, the same row identities and content as today's INSERT
FROM SELECT path would produce under zero-replication-lag (modulo per-item
item_created_at / item_created_by, which are documented out of scope).Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- feat(datasets): pivot to caller-supplied copy-from coords for unchanged-row COPY
Replaces the snapshot boolean shipped in earlier commits with a
caller-supplied (copy_from_dataset_id, copy_from_version_id) pair on
DatasetItemBatch AND DatasetItemChanges. When both are set, the COPY of
unchanged rows (and the edit-via-SELECT-INSERT source rows on
applyDatasetItemChanges) read from that (dataset, version) pair instead of
the destination's just-minted prior version, avoiding the multi-replica
read-after-write window described in OPIK-6601.Why the pivot from "snapshot=true":
The snapshot field made the payload the source of truth for the new
version's row set. For migrate's chained replay, the SDK can't send the
whole 10k-row source v_i on every call (O(N^2) bandwidth). The right shape
is to keep sending only the delta and tell the BE where to read the
carry-forwards from. Two new UUIDs on the wire, no row IDs.Implementation:
- DatasetItemBatch / DatasetItemChanges: add copy_from_dataset_id and
copy_from_version_id (nullable, must be set together). - DatasetItemBatchValidator + inline check in applyDeltaChanges: reject if
only one of the pair is provided. - DatasetItemVersionDAO.applyDelta: signature now takes (datasetId,
newVersionId, ..., copyFromDatasetId, copyFromVersionId) -- baseVersionId
removed since the copy source is now explicit. - DatasetItemVersionDAO.editItemsViaSelectInsert: signature now takes
(sourceDatasetId, sourceVersionId, targetDatasetId, newVersionId, ...).
Edit-via-SELECT-INSERT reads source rows from copy-from coords. - DatasetItemVersionDAO.copyVersionItems: signature now takes
(sourceDatasetId, sourceVersionId, targetDatasetId, targetVersionId, ...). - COPY_VERSION_ITEMS and EDIT_ITEM_VIA_SELECT_INSERT SQL: src.dataset_id ->
:targetDatasetId in the SELECT projection so cross-dataset copies stamp
the destination's dataset_id on inserted rows (was a latent bug -- copies
used src.dataset_id, which worked only when source==target). - createVersionWithDelta classification SELECT (getItemIdsAndHashes) reads
from copy-from coords too -- the classification result is what splits
payload items into "added vs. edited", and reading from a stale base
could mis-route an edit as an add and produce duplicate stable IDs. - applyDeltaChanges: countRowsInVersion (UUID pool sizing),
editItemsViaSelectInsert, and applyDelta all thread copy-from through. - DatasetVersionService and the two intra-dataset copy callers
(filter-based delete, filter-based batch update) pass datasetId as both
source and target -- preserves today's behavior.
Tests (DatasetsResourceTest$CopyFromSourceVersion, 3 tests, all passing):
- createOrUpdate__copyFromSource__unchangedRowsCopiedFromSource: a small
delta sent to destination v_2 with copy_from pointing at a separate
source dataset/version -> destination v_2 contains the delta plus all
of source's items. - createOrUpdate__noCopyFrom__carriesForwardFromDestination: regression
guard for the default path (no copy_from -> reads from destination
prior version, today's behavior). - applyChanges__copyFromSource__readsFromSource: same shape on the
/items/changes endpoint, with a delete and an add, asserting source
items appear in the destination and destSeed is gone.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- fix(datasets): validate copy_from coords + demote DAO INFO to DEBUG
Addresses PR #6867 review (baz-reviewer[bot]):
-
#3304639465 / #3304639472 (High): validate caller-supplied copy-from
coordinates against the workspace before using them. Without this, a
missing or cross-workspace (dataset, version) pair silently produced a
new version with empty carry-forwards (countRowsInVersion returns 0L on
cross-workspace; getItemIdsAndHashes returns empty so all incoming items
classify as adds). Now both applyDeltaChanges and createVersionWithDelta
call versionService.getVersionById(workspaceId, copyDatasetId,
copyVersionId) upfront when copy_from_dataset_id is supplied, which
throws NotFoundException on missing/cross-workspace. Also verify the
resolved version's datasetId matches copy_from_dataset_id (returns 404
if not). -
#3304639484 (Low): demote copyVersionItems' INFO logs to DEBUG so the
service layer (applyDelta) remains the single source of truth for
operational telemetry. Keeps the source/target detail at DEBUG for
diagnostics.
Skipped:
- #3303602672 / #3303602689 (obsolete): refer to the pre-Option-2
snapshot field that no longer exists after commit06e92dd2. - #3304639448 (helper for null-coalesce): extracting a helper for two
pairs of one-linex != null ? x : defaultintroduces a new record
type or tuple return for less code than it removes — over-engineering
per project code-style rules.
Adds an integration test covering the new 404 path on bogus
copy_from_version_id.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
- docs(datasets): keep copy_from API docs user-facing; move impl notes to code comments
Move the OPIK-6696 ticket id and multi-replica read-after-write rationale out of the
published @Operation/@Schema descriptions (which propagate to the OpenAPI spec and
generated SDK docs) into plain // code comments. Relocate the inline -- SQL comments
inside EDIT_ITEM_VIA_SELECT_INSERT and COPY_VERSION_ITEMS to the Java comment blocks
above each constant.- refactor(datasets): DRY copy_from resolve/validate; javadoc endpoint comments
Address review feedback:
- Extract a shared resolveAndValidateCopyFrom helper used by both applyDeltaChanges
and createVersionWithDelta, removing the copy-pasted resolve+validate block and
aligning the gate condition with the values used (validates the caller-supplied
copy_from coords directly instead of the resolved fallbacks). - Share the pair-wise 'set together' check + error text between
DatasetItemBatchValidator and the DatasetItemChanges service-layer guard via
isCopyFromPairConsistent / COPY_FROM_PAIR_MESSAGE. - Convert the endpoint rationale // comments to javadoc on createOrUpdateDatasetItems
and applyDatasetItemChanges.
- chore(datasets): empty commit to re-sync PR #6867 head (no-op; carries 0a9af18b70)
Co-authored-by: Claude Opus 4.7 (1M context) noreply@anthropic.com
Co-authored-by: Thiago Hora thiagoh@comet.com下载附件