Copilot commented on code in PR #788:
URL:
https://github.com/apache/skywalking-banyandb/pull/788#discussion_r2376351992
##########
banyand/trace/syncer.go:
##########
@@ -189,24 +189,10 @@ 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 len(partsToSync) == 0 && !hasSidxParts {
- return nil
- }
// Validate sync preconditions
- if err := tst.validateSyncPreconditions(partsToSync, sidxPartsToSync);
err != nil {
- return err
+ if !tst.needToSync(partsToSync, sidxPartsToSync) {
+ return nil
Review Comment:
The refactored logic removes the error message 'no nodes to sync parts' that
was previously returned when no nodes were available. This makes debugging
harder when sync fails due to missing nodes. Consider adding logging or
returning a descriptive error when needToSync returns false due to no available
nodes.
```suggestion
return fmt.Errorf("no nodes to sync parts")
```
--
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]
