Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compact snapshot stats: Fix incorrect counting for current snapshot #481

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Compact snapshot stats: Fix incorrect counting for current snapshot #481

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions output/compact.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,26 +142,27 @@ func submitCompactSnapshot(ctx context.Context, server *state.Server, collection
if server.CompactLogTime.IsZero() {
server.CompactLogTime = time.Now().Truncate(time.Minute)
server.CompactLogStats = make(map[string]uint8)
} else if time.Now().Sub(server.CompactLogTime) > time.Minute {
var keys []string
for k := range server.CompactLogStats {
keys = append(keys, k)
}
sort.Strings(keys)
details := ""
for i, kind := range keys {
details += fmt.Sprintf("%d %s", server.CompactLogStats[kind], kind)
if i < len(keys)-1 {
details += ", "
}
}
if len(details) > 0 {
logger.PrintInfo("Compact snapshots submitted: " + details)
}
server.CompactLogTime = time.Now().Truncate(time.Minute)
server.CompactLogStats = make(map[string]uint8)
} else {
server.CompactLogStats[kind] = server.CompactLogStats[kind] + 1
if time.Since(server.CompactLogTime) > time.Minute {
var keys []string
for k := range server.CompactLogStats {
keys = append(keys, k)
}
sort.Strings(keys)
details := ""
for i, kind := range keys {
details += fmt.Sprintf("%d %s", server.CompactLogStats[kind], kind)
if i < len(keys)-1 {
details += ", "
}
}
if len(details) > 0 {
logger.PrintInfo("Compact snapshots submitted: " + details)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst looking at this I also considered whether we should reword this to "Submitted compact snapshots" (to be more consistent with the previous wording, and what's used for full snapshots), but I suspect this was intentionally worded this way in #467 since we're talking about what happened in the last minute (vs what just happened), so I think this is okay to leave worded as is.

cc @msakrejda in case you have any wording thoughts on this message.

for context, right now it is:

Compact snapshots submitted: 5 activity, 3 logs

and I'm wondering if it shouldn't be:

Submitted compact snapshots: 5 activity, 3 logs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how it looks like currently, so I can see that a previous wording matches to the full snapshot and might be easier to also scan through.
Alternatively, we could also update the "Submitted snapshot successfully" and make it "Full snapshot submitted", though it doesn't look good 🤔

 I [default] Compact snapshots submitted: 5 activity, 2 logs
 I [default] Compact snapshots submitted: 5 activity, 2 logs
 I [default] Submitted snapshot successfully
 I [default] Compact snapshots submitted: 5 activity, 2 logs

Copy link
Member Author

@lfittl lfittl Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were to change it, I'm thinking this would be most logical:

 I [default] Submitted compact snapshots successfully: 5 activity, 2 logs
 I [default] Submitted full snapshot successfully

Let me make a separate PR for this, and we can discuss there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #482

}
server.CompactLogTime = time.Now().Truncate(time.Minute)
server.CompactLogStats = make(map[string]uint8)
}
}
}

Expand Down
Loading