Copilot commented on code in PR #1133:
URL:
https://github.com/apache/skywalking-banyandb/pull/1133#discussion_r3248354087
##########
banyand/backup/lifecycle/service.go:
##########
@@ -538,7 +547,10 @@ func (l *lifecycleService) buildMigrationReport(p
*Progress) map[string]interfac
// buildSummaryStats creates overall migration statistics.
func (l *lifecycleService) buildSummaryStats(p *Progress)
map[string]interface{} {
- totalGroups := len(p.CompletedGroups) + len(p.DeletedStreamGroups) +
len(p.DeletedMeasureGroups)
+ // total_groups reflects the cycle's scheduled set captured at the top
of
+ // action(); completed_groups counts groups that ran the full
+ // processXxxGroup → MarkGroupCompleted path.
+ totalGroups := len(p.GroupsToProcess)
completedGroups := len(p.CompletedGroups)
Review Comment:
buildSummaryStats() now uses GroupsToProcess as the denominator for
migration_status.total_groups, but still uses len(p.CompletedGroups) for
completed_groups. On resume/retry, CompletedGroups may include groups that are
not in the current GroupsToProcess set (because getGroupsToProcess filters
completed groups), which can make completed_groups > total_groups and
completion_rate > 100. Consider counting only completed groups within
GroupsToProcess (e.g., iterate p.GroupsToProcess and count
p.CompletedGroups[name]).
##########
banyand/backup/lifecycle/service.go:
##########
@@ -436,6 +436,10 @@ func (l *lifecycleService) action(ctx context.Context)
error {
Str("measure_snapshot", measureDir).
Str("trace_snapshot", traceDir).
Msg("created snapshots")
+ // Record the scheduled group set only after snapshots are confirmed so
+ // the report distinguishes "no work this cycle" (total_groups=0) from a
+ // real cycle that processed N groups (total_groups=N).
+ progress.SetGroupsToProcess(getGroupNames(groups))
progress.Save(l.progressFilePath, l.l)
Review Comment:
GroupsToProcess is persisted and later used as the denominator for
migration_status.total_groups. Since it is only set after snapshots are
created, any early-return paths before this point (e.g., when no snapshots are
found and a report is still generated) can emit a report with a stale
GroupsToProcess from a previous run. Consider explicitly resetting
GroupsToProcess to empty for 'no work this cycle' paths, or setting it earlier
and clearing it when snapshots are empty, so total_groups is accurate.
--
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]
