Copilot commented on code in PR #787:
URL:
https://github.com/apache/skywalking-banyandb/pull/787#discussion_r2375880579
##########
banyand/trace/syncer.go:
##########
@@ -189,6 +189,20 @@ func (tst *tsTable) syncSnapshot(curSnapshot *snapshot,
syncCh chan *syncIntrodu
if err != nil {
return err
}
+ if len(partsToSync) == 0 && len(sidxPartsToSync) == 0 {
+ return nil
+ }
+ hasSidxParts := false
+ for _, sidxParts := range sidxPartsToSync {
+ if len(sidxParts) == 0 {
+ continue
+ }
+ hasSidxParts = true
+ break
+ }
+ if !hasSidxParts {
Review Comment:
The logic for checking if sidxPartsToSync has any non-empty parts can be
simplified. The first condition already checks if sidxPartsToSync is empty,
making this additional loop redundant unless there's a specific case where
sidxPartsToSync contains only empty slices.
```suggestion
hasNonEmptySidxParts := false
for _, sidxParts := range sidxPartsToSync {
if len(sidxParts) > 0 {
hasNonEmptySidxParts = true
break
}
}
if len(partsToSync) == 0 && !hasNonEmptySidxParts {
```
##########
banyand/trace/merger.go:
##########
@@ -284,16 +277,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
+ }
Review Comment:
Resources are being released before checking for errors from mergeBlocks. If
mergeBlocks returns an error, the resources are still released but the function
returns early, potentially leaving pm, tf, tt in an undefined state when they
should be used later in the function.
```suggestion
if err != nil {
releaseBlockWriter(bw)
releaseBlockReader(br)
for i := range pii {
releasePartMergeIter(pii[i])
}
return nil, err
}
releaseBlockWriter(bw)
releaseBlockReader(br)
for i := range pii {
releasePartMergeIter(pii[i])
}
```
--
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]