implement time-based upload stats (V2) with configurable retention and auto-cleanup #262
Conversation
| @@ -654,7 +671,23 @@ bool p3FileDatabase::loadList(std::list<RsItem *>& load) | |||
|
|
|||
| if(fu) | |||
There was a problem hiding this comment.
Plz add a date here because this item is deprecated, so that we can remove the code a while when it's not used anymore.
Another option is to completely ignore these "old" items, since upload stats have started very recently anyway.
| { | ||
| mCumulativeUploaded.insert(fu->hash_stats.begin(), fu->hash_stats.end()) ; | ||
| // Migration V1 -> V2: Set timestamp to now | ||
| uint64_t now = time(NULL); |
There was a problem hiding this comment.
use rstime_t which is guarantied to be uint64_t on all platforms.
| @@ -1065,7 +1098,7 @@ uint64_t p3FileDatabase::getCumulativeUploadAll() const | |||
| RS_STACK_MUTEX(mFLSMtx); | |||
| uint64_t total = 0; | |||
| for (auto it = mCumulativeUploaded.begin(); it != mCumulativeUploaded.end(); ++it) | |||
There was a problem hiding this comment.
This looks costly. Depending on whether this method is called many times or not, it may slow down the UI. It's probably better to keep this as a value that is updated in cleanUploadStats(), which you simply return in this method. I don't think we need accuracy beyond 5 secs here anyway.
|
|
||
| struct TimeBasedUploadStat | ||
| { | ||
| uint64_t last_upload_ts; |
There was a problem hiding this comment.
you need a constructor in order to ensure that these values are 0 by default, because they are not initialized when populating the map that contains the hash/values.
| { | ||
| RS_STACK_MUTEX(mFLSMtx); | ||
| mCumulativeUploaded[hash] += size; | ||
| TimeBasedUploadStat& stat = mCumulativeUploaded[hash]; |
There was a problem hiding this comment.
Be careful of initialization here! See comment about the missing constructor of TimeBasedUploadStat.
|
|
||
| void p3FileDatabase::cleanupUploadStats(int days) | ||
| { | ||
| if (days <= 0) return; |
There was a problem hiding this comment.
This function should also probably check that the files are still shared. This may get costly, so see whether that particular check can be done less often and where.
jolavillette
force-pushed
the
ImproveUploadStatistics
branch
from
5de57d7 to
ef03307
Compare
2 participants
this pr works with pr/3093
code and comment by Claude and Gemini
Summary
This PR implements time-based tracking for upload statistics, allowing users to automatically clean up old upload records after a configurable period.
Changes
last_upload_ts) alongside the total byte count. Implemented migration from V1 (bytes only) to V2 (bytes + timestamp).UPLOAD_STATS_RETENTION_DAYSconfiguration key.Verification