Copilot commented on code in PR #744:
URL:
https://github.com/apache/skywalking-banyandb/pull/744#discussion_r2312520371
##########
banyand/trace/merger.go:
##########
@@ -281,16 +281,16 @@ func (tst *tsTable) mergeParts(fileSystem fs.FileSystem,
closeCh <-chan struct{}
}
pm, tf, tt, err := mergeBlocks(closeCh, bw, br)
+ if err != nil {
+ return nil, err
+ }
pm.MinTimestamp = minTimestamp
pm.MaxTimestamp = maxTimestamp
releaseBlockWriter(bw)
releaseBlockReader(br)
for i := range pii {
releasePartMergeIter(pii[i])
}
- if err != nil {
- return nil, err
- }
pm.mustWriteMetadata(fileSystem, dstPath)
Review Comment:
Moving the error check before setting MinTimestamp and MaxTimestamp could
cause issues if pm is nil when mergeBlocks returns an error. The code should
handle the nil pm case or ensure pm is valid before accessing its fields.
##########
banyand/trace/block.go:
##########
@@ -316,7 +321,6 @@ func mustSeqReadSpansFrom(spans [][]byte, sm *dataBlock,
count int, reader *seqR
if uint64(len(src)) < spanLen {
logger.Panicf("insufficient data for span: need %d
bytes, have %d", spanLen, len(src))
}
Review Comment:
Similar to the previous issue, removing `spans[i] = spans[i][:0]` will cause
data accumulation and memory leaks in sequential span reading.
```suggestion
}
spans[i] = spans[i][:0]
```
##########
banyand/trace/block.go:
##########
@@ -292,7 +298,6 @@ func mustReadSpansFrom(spans [][]byte, sm *dataBlock, count
int, reader fs.Reade
if uint64(len(src)) < spanLen {
logger.Panicf("insufficient data for span: need %d
bytes, have %d", spanLen, len(src))
}
- spans[i] = spans[i][:0]
spans[i] = append(spans[i], src[:spanLen]...)
Review Comment:
Removing the line `spans[i] = spans[i][:0]` means that spans[i] will keep
accumulating data from previous calls instead of being reset. This will cause
memory leaks and incorrect data accumulation.
--
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]
