trufflehog
trufflehog copied to clipboard
[THOG-712] - Monotonically increasing progress counter
Initial implementation to make progress increment monotically. The main goal i'm trying to accomplish here is separating out progress completion from source. With our current implementation the source updates the progress, but that tends to happen in goroutines and using a loop variable. This can lead to buggy situations and incorrect progress completion values while scanning.
- Add Counter struct to Progress struct that monotonically increases. This will keep track of each scan for each source. This will also be safe to use in goroutines.
- Refactor SetProgressComplete into two separate methods:
UpdateandComplete. Update works on updating progress during a scan and Complete gets called at the end of the scan. (idk if this is too much refactoring, it just seemed a little more obvious to me than explicitly passing in the len of repos or buckets etc. This abstracts that away and just uses the total count from the counter and sets the progress explicitly) - Include multiple fields in the counter to keep track of successful scan counts and total scan counts. This is so we can correctly keep track of both the percent complete (using the successCnt) and also update the progress in the logs when concurrently scanning.
- TotalCnt and SuccessCnt should have the same end value if all scans succeed. They only differ during scans to prevent multiple scans from having the same counter value prior to successfully finishing. Once a scan successfully finishes scanning the SuccessCnt is also incremented.
- Both TotalCnt and SuccessCnt and safe for concurrent use.
Overall I think it looks good. A few thoughts:
- There's possible confusion with the function names
IncSuccessandComplete - We need to decide what percent complete means -- successful completions? Work completed? I can see arguments to both, though I think "work completed" is slightly better (mostly because we're using this to indicate how many items we've gone through)
Overall I think it looks good. A few thoughts:
- There's possible confusion with the function names
IncSuccessandComplete- We need to decide what percent complete means -- successful completions? Work completed? I can see arguments to both, though I think "work completed" is slightly better (mostly because we're using this to indicate how many items we've gone through)
Yea so I just had it as Complete since it's a method on Progress so complete means we are done progressing. I guess then we need to clarify progress a bit more? It sounds and reads a bit weird to have progress.WorkComplete? It's almost like another noun suddenly gets injected into the meaning.
If everyone things that seems better i'm more than happy to go with that approach.
Maybe we should get another opinion on how to calculate percent complete before merging.