You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "hanahmily (via GitHub)" <gi...@apache.org> on 2026/08/11 06:04:04 UTC
[PR] Bound the trace merge drop-set with a retain-instead-of-drop ceiling [skywalking-banyandb]
hanahmily opened a new pull request, #1277:
URL: https://github.com/apache/skywalking-banyandb/pull/1277
### Bound the trace merge drop-set, plus the sampled-merge optimizations behind it
Design docs: [`trace-drop-set-bounding.md`](https://github.com/apache/skywalking-banyandb/blob/hapi-trace-merge/docs/design/trace-drop-set-bounding.md) (rationale, measurements, rejected alternatives) and [`trace-drop-set-bounding-plan.md`](https://github.com/apache/skywalking-banyandb/blob/hapi-trace-merge/docs/design/trace-drop-set-bounding-plan.md) (per-ticket boundaries and tests).
#### The problem
The merge-wide dropped-trace-ID collector from the in-merge retention filter was unbounded. A shard's first finalize round selects **every** cooled part into one merge, so the set scales with shard size. Measured on the real structure with 42-byte service-prefixed IDs:
| Dropped traces | Live heap | Reserved heap |
| ---: | ---: | ---: |
| 1,000,000 | 71 MiB | 163 MiB |
| 4,000,000 | 281 MiB | 615 MiB |
| 16,000,000 | **1.11 GiB** | **2.27 GiB** |
Reserved heap is ~2x live because `ids` grows by append doubling and the lookup index is one contiguous allocation. Reserved heap is what terminates a container.
#### The approach
The ceiling is applied to the sampling **decision**, not to the pruning predicate. When a merge's drop set is full, every further proposed drop is *retained* instead of recorded. Recording and dropping therefore stop at the same instant, so the set stays complete with respect to the drops actually performed — which is why nothing downstream changes: `keepEncoded`, `filterBlockPointer`, the sidx merge signature, the output part's `traceIDFilter`, the finalize round machinery and **every query path** are untouched. Phase 6's hard invariant — *"Secondary-index lookup remains exact. No probabilistic structure can authorize a deletion."* — is preserved verbatim.
One budget for every merge, finalize included, resolved from the memory protector. Bytes-per-entry is derived from the observed trace-ID length rather than a fixed constant, so long service-prefixed IDs cannot overshoot the budget.
**Rejected: bloom-filter pruning.** Releasing the exact set and pruning with the output part's retained-ID filter would have bounded memory with *no* deletion loss, and a bloom has no false negatives so deletions stay exact. It was rejected because a surviving index entry for a dropped trace is **not** query-invisible on the default engine: the pushed-down limit (`limit + offset`) caps Phase 1 on sidx-derived trace IDs *before* the core scan, and nothing backfills — so an orphan inside the top-N window silently shortens the result set. See §8 of the design doc; the same finding rules out "stop pruning" as a fallback.
#### The accepted cost
Bounded deletion. A capped finalize round leaves traces undeleted, revisited only by the existing re-round path, capped at `max_finalize_rounds` (8) per shard for its lifetime — roughly 2.7M traces per shard at 4 GiB / 8 CPUs. Finalize I/O is unchanged because the round count is unchanged. Four ways to lift this (per-lane finalize ceiling, continuation rounds, sampling watermark, bloom pruning) are specified and deferred in §9 with their prerequisites.
#### Observability
Because the cost is otherwise silent, the instruments are part of this change rather than a follow-up: `pipeline_drop_set_budget_bytes`, a `pipeline_drop_set_entries` histogram emitted **whether or not** a merge caps (so headroom is visible before the ceiling bites), shard-labeled `pipeline_traces_retained_by_ceiling` / `pipeline_merges_ceiling_reached`, and `finalize_rounds` / `finalize_terminal` per shard from the finalize scan pre-filter — the only place a terminal shard is still visited.
#### Also in this PR
Earlier sampled-merge work on the same branch: drop-path and projected-sampling optimizations, a benchmark readiness alignment, and the merge-optimization plan's recorded validation results.
#### Validation status — please read
The ceiling is **live** on every merge. Unit and race coverage is complete (full `banyand/trace` suite and `-race` green, `make pre-push` green), including strict set-equality between each sidx and the output part's trace IDs so both an orphan and a missing entry fail.
**Workload-level validation is outstanding.** It rides the Phase 10 SkyWalking acceptance run as two runs: one at default settings, which measures how much headroom real traffic has against the ceiling, and one at `--allowed-bytes=256MiB`, which lowers the ceiling enough for real traffic to reach it and is the only run that exercises the capped path. The containerised controlled run was considered and rejected: at canonical resources the ceiling sits near 1.34M traces while the existing frozen selection holds 33,353 — ~40x too small to make a merge cap.
Activation is two lines (`merger.go`, `finalizer.go`, both carrying `resolveDropSetBudget`); reverting them returns the mechanism to inert with no code removed.
- [x] If this is non-trivial feature, paste the links/URLs to the design doc.
- [x] Update the documentation to include this new feature.
- [x] Tests(including UT, IT, E2E) are added to verify the new feature.
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-banyandb/blob/main/CHANGES.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bound the trace merge drop-set with a retain-instead-of-drop ceiling [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily merged PR #1277:
URL: https://github.com/apache/skywalking-banyandb/pull/1277
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org