github-featured / comet-ml--opik

发布

0 发布 8,340 Git 标签

RSS 订阅源
  • 2.2.18-6246 b8df6acb0c
    比较

    [OPIK-7455] [BE] feat: retarget trace mutation DAOs to traces_local behind a wrap toggle (#7713)

    frostbyte_neo 发布于 2026-08-04 13:23:41 +00:00

    • [OPIK-7455] [BE] feat: retarget trace mutation DAOs to traces_local behind a wrap toggle

    The traces cutover's final, deferred step wraps traces as a Distributed table
    over the traces_local shard. A Distributed table supports SELECT/INSERT but
    not mutations (ClickHouse: DELETE -> code 36, ALTER ... DELETE -> code 48), so
    every trace DELETE would return 500 the instant the wrap is applied. This ships
    the code prerequisite before the wrap is enabled.

    TraceDAO's three mutation paths — DELETE_BY_PROJECT_ID_TRACE_ID_PAIRS
    (delete-by-id), DELETE_FOR_RETENTION, and the dynamic deleteForRetentionBounded
    — now carry both table names in an <if(distributed_wrap)>traces_local<else> traces<endif> branch, gated by the new
    databaseAnalyticsDataModel.tracesDistributedWrapEnabled toggle. Default false
    keeps mutations on traces (still a MergeTree pre-wrap); set true in lockstep
    with applying the wrap to route them at traces_local. Reads and inserts always
    stay on the Distributed traces. A repo-wide audit confirmed these are the only
    DELETE/ALTER/OPTIMIZE paths on traces.

    The toggle is wired through config.yml, config-test.yml, docker-compose, and the
    Helm chart (values, configmap, and its unittest), mirroring the sibling cutover
    toggles so operators can actually set it. The accessor Javadoc documents the
    general rule (future mutation/ALTER/OPTIMIZE paths and migrations switch tables
    the same way) and why single-shard mutations need no ON CLUSTER; the cutover
    runbook and exchange_and_wrap.sh name the flag as the concrete prerequisite for
    the wrap.

    Add TracesDistributedWrapMutationTest: a black-box suite on dedicated containers
    that builds the post-wrap Distributed topology and proves, through the public
    trace API, that delete-by-id and both retention paths run against traces_local
    while reads/inserts flow through the Distributed traces, with a guard test
    asserting a direct mutation on the Distributed traces is rejected.

    Implements OPIK-7455.

    • chore(helm): regenerate chart README for tracesDistributedWrapEnabled

    The helm-docs pre-commit hook failed in CI because values.yaml gained the
    tracesDistributedWrapEnabled entry without the generated README values table
    being refreshed. Regenerated via jnorwood/helm-docs (same image as CI).

    • refactor(traces): centralize wrap-table ST attribute + document rollback flag reset

    Address Baz PR review:

    • Extract addTracesMutationTable(ST) so the <if(distributed_wrap)> branch is
      applied from one place by both ST delete templates (the flag decision already
      lived in the single tracesDistributedWrapEnabled() accessor).
    • Document the symmetric rollback prerequisite: Stage C makes traces a
      MergeTree again and parks traces_local, so tracesDistributedWrapEnabled must
      be set back to false before backends resume — added to the runbook Rollback
      section and the 000004_rollback_stage_c comment.
    • docs(cutover): refresh runbook references after OPIK-7483 and the flag reframing

    Address JetoPistola's review (documentation-only, non-blocking):

    • Delta/replay section no longer describes the workspace-scoped, empty-project
      delete fallback as current: OPIK-7483 removed it, so TraceService.delete always
      carries project_id and the empty-project replay branch now covers only legacy
      pre-OPIK-7483 bridge rows. Drops the stale TraceDAO.DELETE_BY_ID reference.
    • "once the sharding-aware DAO has shipped" -> the retarget flag being live across
      the fleet, matching the flag-based framing this PR introduced.
    • Go/No-Go exit criteria phrased the same way (retarget flag live and healthy).
    • docs(cutover): carve schema ALTERs out of the general rule + note partition-metrics relabel

    Address Thiago's review (documentation-only, non-blocking):

    • The "target traces_local" general rule was wrong for schema migrations. Split
      it by kind across all five sites (README, DatabaseAnalyticsDataModelConfig,
      TraceDAO accessor, config.yml, config-test.yml): row mutations + MATERIALIZE
      COLUMN / ADD INDEX / MODIFY TTL go to traces_local only (the Distributed traces
      rejects them, code 36/48); ADD/DROP/MODIFY COLUMN must hit both traces_local and
      the Distributed traces, since the wrapper takes them as metadata-only and
      traces_local alone leaves reads unable to see the column (code 47).
    • Note the monitoring consequence in the flip checklist and its Stage C inverse:
      the opik.clickhouse.partition.* parts gauges relabel traces<->traces_local at
      the wrap/rollback while the LWD-mask gauge stays labelled traces.
    • refactor(traces): rename addTracesMutationTable -> selectTracesMutationTable

    Address Baz: the helper adds the distributed_wrap template attribute (which
    selects the mutation table via the branch) rather than "adding a table";
    the intent-based name reads correctly at the two call sites.

    下载附件