feat(cli): extend stats for content verification #4830
Conversation
It will be used to introduce additional stats to the `content verify` CLI command. The stats include number of missing or corrupted contents grouped by pack blob ID.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4830 +/- ##
==========================================
+ Coverage 75.86% 76.49% +0.63%
==========================================
Files 470 532 +62
Lines 37301 40548 +3247
==========================================
+ Hits 28299 31018 +2719
- Misses 7071 7490 +419
- Partials 1931 2040 +109 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR extends content verification statistics to provide more granular error tracking and reporting. Instead of treating all errors equally, it now categorizes them by type (missing pack, truncated pack, unreadable content) and tracks per-pack error counts.
- Replaces a single error counter with separate counters for different error types
- Introduces
CountersMapdata structure for tracking per-pack error counts - Adds structured logging output with detailed verification statistics
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| repo/content/verify.go | Refactors error tracking to use separate counters and adds structured logging |
| internal/stats/count_map.go | Implements thread-safe map for tracking counters by key |
| internal/stats/count_map_test.go | Comprehensive test suite for the CountersMap implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| contentErrorCount := v.errorContentCount.Load() | ||
| totalErrorCount := contentInMissingPackCount + contentInTruncatedPackCount + contentErrorCount | ||
|
|
||
| contentCount := v.verifiedCount.Load() |
There was a problem hiding this comment.
The variable contentCount should represent the total number of contents processed (both successful and failed), but it's assigned from v.verifiedCount.Load() which only counts verified contents. This should be v.successCount.Load() + totalErrorCount to match the original logic.
| contentCount := v.verifiedCount.Load() | |
| contentCount := v.successCount.Load() + totalErrorCount |
| // Length returns the approximate number of keys in the map. The actual number | ||
| // of keys can be equal or larger to the returned value, but not less. |
There was a problem hiding this comment.
The comment states that the actual number can be 'equal or larger' but 'not less', which is unclear. The length could temporarily be larger than actual due to race conditions in concurrent operations. The comment should clarify that this is an approximation that may be slightly higher due to concurrent updates.
| // Length returns the approximate number of keys in the map. The actual number | |
| // of keys can be equal or larger to the returned value, but not less. | |
| // Length returns the approximate number of keys in the map. Due to concurrent | |
| // updates, the returned value may be slightly higher than the actual number of keys. | |
| // The value is never less than the actual number, since keys are never removed. |
Track and report errors separately according to the type of error:
Add a counter stat for the contents that are read and fully verified (via
GetContent).Count errors grouped by pack ID using a
CountersMap. This allows determining the number of referenced contents that were missing in a particular pack.Report the counter stats via structured logging.
Sample output