[I] [Bug] BanyanDB trace pipeline: MERGE-event sampler verdict is fragment-local — merge_grace guarantees freshness, not trace completeness [skywalking]

["wu-sheng (via GitHub)" <gi...@apache.org>]

wu-sheng opened a new issue, #13963:
URL: https://github.com/apache/skywalking/issues/13963

   > Code paths in this issue are in `apache/skywalking-banyandb`. Umbrella: #13634.
   
   Found while reviewing apache/skywalking-banyandb#1234. Companion to the split-block contract issue.
   
   ## Problem
   
   At `PIPELINE_EVENT_MERGE` the sampler is evaluated on whatever subset of parts the LSM compaction policy happened to select. `merge_grace` bounds **freshness**, not **completeness**:
   
   ```go
   // banyand/trace/merger.go:368-375
   graceNs := lookupMergeGrace(tst.group)
   if graceNs <= 0 { graceNs = int64(tst.option.mergeGraceDefault) }
   if isMergeHot(parts, graceNs, time.Now().UnixNano()) {
       return nil          // whole merge runs unfiltered
   }
   ```
   
   `isMergeHot` (`merger.go:771-777`) only asks whether any part in the merge is younger than the grace window. Part selection is `mergePolicy.getPartsToMerge` (`banyand/trace/merger_policy.go:53-100`), a size-tiered search for lowest write amplification with no notion of trace membership.
   
   A trace whose segments straddled a flush boundary sits in parts P1/P2/P3, all long past `merge_grace`. The policy picks {P2, P3}. The sampler sees only those rows, finds no error and no slow envelope, and drops them. The error segment in P1 survives alone — a truncated trace. Reverse the selection order and the error evidence itself is destroyed, after which the later pass drops the remainder.
   
   Only the finalizer selects **all** cooled parts (`banyand/trace/finalizer.go:85-108`, a full-shard rewrite), so only `PIPELINE_EVENT_FINALIZE` evaluates something resembling a whole trace — and it is **off by default** while MERGE is on:
   
   ```go
   // banyand/trace/metadata.go:296-311
   // PIPELINE_EVENT_MERGE installs no samplers. An empty enabled_events list defaults ...
   // pass. Unlike MERGE, an empty enabled_events list does NOT default finalize on:
   ```
   
   Out of the box, a whole-trace retention policy therefore runs only at the fragment-level pass.
   
   ## Design-doc divergence
   
   `docs/design/post-trace-pipeline.md` specifies a **per-trace** maturity gate — lines 325, 363, 568, 581:
   
   > The filter therefore evaluates a trace only once its latest span timestamp is older than `now − merge_grace` … traces newer than that frontier are passed through the merge unchanged.
   
   The implementation is a **per-merge, part-level** gate that skips filtering for the entire merge when any part is fresh. That is stricter, so the "never drop a still-growing trace" property does hold — but the mechanism the document describes does not exist, and §7.1 point 6 then draws the wrong conclusion from it:
   
   > Because a trace is dropped only after `merge_grace` has elapsed since its last span … the drop is final rather than premature.
   
   "Nothing more is arriving" is not "everything already written is visible in this merge".
   
   ## Suggested direction
   
   1. Document plainly that a MERGE-event verdict is fragment-local, and that whole-trace policies belong at `PIPELINE_EVENT_FINALIZE`.
   2. Consider defaulting FINALIZE on (or refusing a MERGE-only configuration for plugins that declare a whole-trace policy).
   3. Correct §7.1 to describe the part-level gate that is actually implemented.
   
   A true fix at merge time needs a trace→part index so the engine can tell whether a trace's rows are fully contained in the merged set; short of that, fragment-local is the honest description.
   
   Related: apache/skywalking-banyandb#1234.
   


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] BanyanDB trace pipeline: MERGE-event sampler verdict is fragment-local — merge_grace guarantees freshness, not trace completeness [skywalking]

["hanahmily (via GitHub)" <gi...@apache.org>]

hanahmily closed issue #13963: [Bug] BanyanDB trace pipeline: MERGE-event sampler verdict is fragment-local — merge_grace guarantees freshness, not trace completeness
URL: https://github.com/apache/skywalking/issues/13963


-- 
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