apache / apache/skywalking

[Chore] Replace the multi-group Measure row merger with a batch merger

Open
#14,068 0 comments 0 reactions 0 assignees View on GitHub
database
Dominant language
Java
Stars
25k
Forks
6.6k
Avg merge
10h 5m
Merged PRs (30d)
16

Description

## Summary

Merge vectorized Measure results without first converting them into individual `InternalDataPoint` objects. Follow-up to #14066 under #13998.

## Size audit and dependencies

Classification: **tracking parent**, not an executable leaf or automated implementation task.
Boundary: the production multi-group Measure caller of `MergeGroupMIterators` and its `sortableDataPoints` / `sortedMIterator` stack.
This spans batch consumption, caller/egress migration, schema alignment, ordering/version deduplication, and resource ownership. Decompose after the legacy removal merges.
Blocked by: #14066 via BanyanDB PR apache/skywalking-banyandb#1326.

## Requirements

R1. Consume and merge per-group batches directly; do not reconstruct the row merger internally.
R2. Preserve requested ordering, version-based deduplication, schema alignment, and pagination.
R3. Materialize protobuf only where the response boundary requires it; preserve tracing and response compatibility.
R4. Release consumed batches correctly on completion, cancellation, and errors.

## Concrete oracle

Two groups supply these records, with duplicate versions split across batches:

```text
timestamp=10, SID=7, version=1, value=100
timestamp=10, SID=7, version=3, value=300
timestamp=20, SID=8, version=1, value=200
```

Ascending output is `(10,7,3,300)` followed by `(20,8,1,200)`. Descending reverses that order. Ascending with offset 1 and limit 1 returns only `(20,8,1,200)`.

## Completion criteria and RED-test direction

- [ ] The real processor multi-group caller consumes the batch-native merger.
- [ ] Tests assert the exact oracle across batch boundaries, plus index ordering and supported schema alignment.
- [ ] A path-sensitive production-caller regression fails if row iteration/materialization is restored. Output assertions alone are insufficient: the existing merger already produces these answers.
- [ ] Each executable child identifies its public boundary and failing test before implementation; no helper-only foundation PR counts as completion.
- [ ] Cancellation/error tests verify resource release; benchmark comparisons report allocations without claiming a new resource bound that has not been proved.

Packages: `banyand/query`, `pkg/query/logical/measure`, `pkg/query/vectorized/measure`.

```sh
go test ./banyand/query ./pkg/query/logical/measure ./pkg/query/vectorized/measure/...
go test ./test/integration/standalone/query/... ./test/integration/distributed/query/...
```

## Out of scope

Changing ordinary Top or GroupBy semantics; the internal TopN pre-aggregation RPC; schema-evolved aggregate partial reduction (separate workstream).

Parent: #13998.

## Compatibility and review policy

- Preserve existing on-disk and wire formats and mixed-version compatibility. Any necessary protocol change requires a separately reviewed compatibility contract.
- Use explicit, hand-calculated expected results; do not depend on the removed row engine as the test oracle.
- Human review enforces the no-new-row-path policy. A row-call-site lint guard is not required.

Contributor guide

Open the contributing guide

Research direction

Start with the production multi-group Measure caller of MergeGroupMIterators and the sortableDataPoints/sortedMIterator stack in banyand/query, pkg/query/logical/measure, and pkg/query/vectorized/measure. Run the listed Go tests and inspect the RED-test direction before implementation. Done means the caller uses the batch-native merger, preserves ordering, deduplication, schema alignment, and pagination, and tests cover boundaries, cancellation, errors, and resource release.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
observability
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.