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]

Reply via email to