mrproliu opened a new pull request, #1205:
URL: https://github.com/apache/skywalking-banyandb/pull/1205

   ## Summary
   
   Fixes an intermittent **data-corruption bug** in the stream slow-path 
migration that silently writes wrong tag values into migrated parts. It first 
surfaced as the flaky `TestMigrationSlowPathMultiStreamElementIndexRebuild` in 
the scheduled Flaky Tests job 
(https://github.com/apache/skywalking-banyandb/actions/runs/28476575201/job/84402810627#step:6:1),
 but the corruption is real (not a test artifact) and reproduces without 
`-race`.
   
   ## Root cause
   
   The slow-copy path (`slowCopyOneStreamPart`) allocates `tagValue`s from a 
per-call arena (`streamSlowCopyArena.allocTagValue`), which returns pointers 
**into the arena's slab** (`&a.tvs[i]`). Those elements are then freed with 
`releaseElements(el)`, which walks down to `releaseTagValue` and **puts the 
arena-owned `*tagValue` pointers into the global `tagValuePool`**.
   
   The same `tagValue` memory now has two owners — the arena (which 
reuses/overwrites its slab) and `tagValuePool` (which hands it out to other 
callers). A later `generateTagValue` (any stream write) receives that aliased 
object while the arena concurrently overwrites it, so a tag column ends up 
holding another column's value (e.g. streamA's `status` becomes streamB's 
`code="200"`). The corrupt part is then written to disk; migration and queries 
read it back faithfully, so the element index loses terms.
   
   This is an ownership violation / use-after-free, not a missing reset. It 
only triggers when the slow-copy migration runs alongside 
`generateTagValue`-based writes in the same process, and is timing-dependent 
(~0.2% without `-race`, ~10-17% with `-race`).
   
   ## Fix
   
   Do not return arena-owned tagValues to the global pool. Add 
`releaseArenaElements`, which recycles the `elements` shell but skips releasing 
its (arena-owned) tagValues and clears the `tagFamilies` entries so no arena 
pointers linger. Use it at the two slow-copy release sites in 
`migration_copy.go`.
   
   One file, ~20 lines. No behavior change for the normal write path.
   
   ## Testing
   
   - Control (same instant corruption detector, only variable = the fix):
     - **without** fix: crashes at iteration 15 (`GOMAXPROCS=2 -race`) and 23 
(default cores `-race`) within ~15s — `status` column holds a `code` value.
     - **with** fix: 5,000 iterations clean in both configs.
   - **50,000** iterations, no failures, in both `GOMAXPROCS=2 -race` (matches 
the GitHub CI runner) and default cores without `-race`.
   - All migration / slow-path / dump / copy tests pass under `-race`; `gofmt` 
/ `go vet` clean.
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace 
the issue number. Fixes apache/skywalking#<issue number>.
   - [ ] 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to