Skip to content

implement time-based upload stats (V2) with configurable retention and auto-cleanup #262

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

Open
jolavillette wants to merge 1 commit into
base: master
Choose a base branch
from
Open

implement time-based upload stats (V2) with configurable retention and auto-cleanup #262

Show file tree
Hide file tree
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
101 changes: 95 additions & 6 deletions src/file_sharing/p3filelists.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ p3FileDatabase::p3FileDatabase(p3ServiceControl *mpeers)
mLastDataRecvTS = 0 ;
mTrustFriendNodesForBannedFiles = TRUST_FRIEND_NODES_FOR_BANNED_FILES_DEFAULT;
mLastPrimaryBanListChangeTimeStamp = 0;
mUploadStatsRetentionDays = 0;

// This is for the transmission of data

Expand Down Expand Up @@ -197,6 +198,7 @@ int p3FileDatabase::tick()
if(mLastCleanupTime + 5 < now)
{
cleanup();
cleanupUploadStats(mUploadStatsRetentionDays);
mLastCleanupTime = now ;
}

Expand Down Expand Up @@ -379,12 +381,12 @@ cleanup = true;

{
RS_STACK_MUTEX(mFLSMtx) ;
RsFileListsUploadStatsItem *item = nullptr;
RsFileListsUploadStatsItemV2 *item = nullptr;

for(auto it(mCumulativeUploaded.begin());it!=mCumulativeUploaded.end();++it)
{
if(item == nullptr)
item = new RsFileListsUploadStatsItem ;
item = new RsFileListsUploadStatsItemV2 ;

item->hash_stats.insert(*it);

Expand Down Expand Up @@ -497,6 +499,15 @@ cleanup = true;

kv.key = IGNORE_LIST_FLAGS_SS; kv.value = s; rskv->tlvkvs.pairs.push_back(kv);
}
{
std::string s;
rs_sprintf(s, "%d", mUploadStatsRetentionDays);

RsTlvKeyValue kv;
kv.key = UPLOAD_STATS_RETENTION_DAYS_SS;
kv.value = s;
rskv->tlvkvs.pairs.push_back(kv);
}

/* Add KeyValue to saveList */
sList.push_back(rskv);
Expand Down Expand Up @@ -618,6 +629,12 @@ bool p3FileDatabase::loadList(std::list<RsItem *>& load)
if(sscanf(kit->value.c_str(),"%d",&t) == 1)
max_share_depth = (uint32_t)t ;
}
else if(kit->key == UPLOAD_STATS_RETENTION_DAYS_SS)
{
int t=0;
if(sscanf(kit->value.c_str(),"%d",&t) == 1)
mUploadStatsRetentionDays = t;
}

delete *it ;
continue ;
Expand Down Expand Up @@ -654,7 +671,23 @@ bool p3FileDatabase::loadList(std::list<RsItem *>& load)

if(fu)
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

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);
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

use rstime_t which is guarantied to be uint64_t on all platforms.

RsDbg() << "UPLOADSTATS Migrating V1 stats (count: " << fu->hash_stats.size() << ") to V2";
for(auto const& [hash, bytes] : fu->hash_stats)
{
TimeBasedUploadStat& stat = mCumulativeUploaded[hash];
stat.total_bytes = bytes;
stat.last_upload_ts = now;
}
}

RsFileListsUploadStatsItemV2 *fu2 = dynamic_cast<RsFileListsUploadStatsItemV2*>(*it) ;

if(fu2)
{
RsDbg() << "UPLOADSTATS Loading V2 stats (count: " << fu2->hash_stats.size() << ")";
mCumulativeUploaded.insert(fu2->hash_stats.begin(), fu2->hash_stats.end()) ;
}

delete *it ;
Expand Down Expand Up @@ -1056,7 +1089,7 @@ uint64_t p3FileDatabase::getCumulativeUpload(const RsFileHash& hash) const
RS_STACK_MUTEX(mFLSMtx);
auto it = mCumulativeUploaded.find(hash);
if (it != mCumulativeUploaded.end())
return it->second;
return it->second.total_bytes;
return 0;
}

Expand All @@ -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)
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

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.

total += it->second;
total += it->second.total_bytes;
return total;
}

Expand All @@ -1078,15 +1111,71 @@ uint64_t p3FileDatabase::getCumulativeUploadNum() const
void p3FileDatabase::addUploadStats(const RsFileHash& hash, uint64_t size)
{
RS_STACK_MUTEX(mFLSMtx);
mCumulativeUploaded[hash] += size;
TimeBasedUploadStat& stat = mCumulativeUploaded[hash];
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

Be careful of initialization here! See comment about the missing constructor of TimeBasedUploadStat.

stat.total_bytes += size;
stat.last_upload_ts = time(NULL);

// RsDbg() << "UPLOADSTATS add stats: " << hash << " + " << size << " bytes. Total: " << stat.total_bytes << " ts: " << stat.last_upload_ts;
IndicateConfigChanged(RsConfigMgr::CheckPriority::SAVE_OFTEN);
}

void p3FileDatabase::clearUploadStats()
{
RS_STACK_MUTEX(mFLSMtx);
RsDbg() << "UPLOADSTATS clearing all stats";
mCumulativeUploaded.clear();
}

void p3FileDatabase::cleanupUploadStats(int days)
{
if (days <= 0) return;
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

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.


RS_STACK_MUTEX(mFLSMtx);
time_t cutoff = time(NULL) - (days * 24 * 3600);
uint32_t removed_count = 0;

// RsDbg() << "UPLOADSTATS cleanup stats older than " << days << " days (cutoff: " << cutoff << ")";

for (auto it = mCumulativeUploaded.begin(); it != mCumulativeUploaded.end(); )
{
if (it->second.last_upload_ts < (uint64_t)cutoff)
{
RsDbg() << "UPLOADSTATS removing expired stat: " << it->first << " (ts: " << it->second.last_upload_ts << ")";
it = mCumulativeUploaded.erase(it);
removed_count++;
}
else
{
++it;
}
}
if (removed_count > 0)
{
RsDbg() << "UPLOAD cleanup removed " << removed_count << " entries.";
IndicateConfigChanged(RsConfigMgr::CheckPriority::SAVE_OFTEN);
}
}

void p3FileDatabase::setUploadStatsRetentionDays(int days)
{
if (mUploadStatsRetentionDays != days)
{
mUploadStatsRetentionDays = days;
RsDbg() << "UPLOADSTATS setting retention days to: " << days;
IndicateConfigChanged(RsConfigMgr::CheckPriority::SAVE_OFTEN);

// Trigger cleanup immediately if days > 0
if (days > 0)
{
cleanupUploadStats(days);
}
}
}

int p3FileDatabase::getUploadStatsRetentionDays() const
{
return mUploadStatsRetentionDays;
}
bool p3FileDatabase::removeExtraFile(const RsFileHash& hash)
{
bool ret = false;
Expand Down
13 changes: 12 additions & 1 deletion src/file_sharing/p3filelists.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
#include "ft/ftsearch.h"
#include "ft/ftextralist.h"
#include "retroshare/rsfiles.h"
#include "file_sharing/rsfilelistitems.h"
#include "services/p3service.h"
#include "util/rstime.h"
#include "file_sharing/hash_cache.h"
Expand Down Expand Up @@ -177,6 +178,12 @@ class p3FileDatabase: public p3Service, public p3Config, public ftSearch //, pub
virtual uint64_t getCumulativeUploadNum() const;
virtual void addUploadStats(const RsFileHash& hash, uint64_t size);
void clearUploadStats();
void cleanupUploadStats(int days);

void setUploadStatsRetentionDays(int days);
int getUploadStatsRetentionDays() const;



// interface for hash caching

Expand Down Expand Up @@ -301,7 +308,11 @@ class p3FileDatabase: public p3Service, public p3Config, public ftSearch //, pub
bool mBannedFileListNeedsUpdate;
rstime_t mLastPrimaryBanListChangeTimeStamp;

std::map<RsFileHash, uint64_t> mCumulativeUploaded;
/**
* @brief Map of uploaded files statistics (bytes + timestamp)
*/
std::map<RsFileHash, TimeBasedUploadStat> mCumulativeUploaded;
int mUploadStatsRetentionDays;

void locked_sendBanInfo(const RsPeerId& pid);
void handleBannedFilesInfo(RsFileListsBannedHashesItem *item);
Expand Down
14 changes: 13 additions & 1 deletion src/file_sharing/rsfilelistitems.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,16 @@
******************************************************************************/
#include "serialiser/rsbaseserial.h"

#include "serialiser/rstypeserializer.h"
#include <serialiser/rstypeserializer.h>

#include "file_sharing/rsfilelistitems.h"

template<> void RsTypeSerializer::serial_process<TimeBasedUploadStat>(RsGenericSerializer::SerializeJob j, RsGenericSerializer::SerializeContext& ctx, TimeBasedUploadStat& v, const std::string& /*name*/)
{
serial_process(j, ctx, v.last_upload_ts, "last_upload_ts");
serial_process(j, ctx, v.total_bytes, "total_bytes");
}

void RsFileListsSyncRequestItem::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx)
{
RsTypeSerializer::serial_process (j,ctx,entry_hash,"entry_hash") ;
Expand Down Expand Up @@ -55,6 +61,11 @@ void RsFileListsUploadStatsItem::serial_process(RsGenericSerializer::SerializeJo
RsTypeSerializer::serial_process(j,ctx,hash_stats,"hash_stats") ;
}

void RsFileListsUploadStatsItemV2::serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx)
{
RsTypeSerializer::serial_process(j,ctx,hash_stats,"hash_stats") ;
}

RsItem *RsFileListsSerialiser::create_item(uint16_t service,uint8_t type) const
{
if(service != RS_SERVICE_TYPE_FILE_DATABASE)
Expand All @@ -67,6 +78,7 @@ RsItem *RsFileListsSerialiser::create_item(uint16_t service,uint8_t type) const
case RS_PKT_SUBTYPE_FILELISTS_BANNED_HASHES_ITEM: return new RsFileListsBannedHashesItem();
case RS_PKT_SUBTYPE_FILELISTS_BANNED_HASHES_CONFIG_ITEM: return new RsFileListsBannedHashesConfigItem();
case RS_PKT_SUBTYPE_FILELISTS_UPLOAD_STATS_ITEM: return new RsFileListsUploadStatsItem();
case RS_PKT_SUBTYPE_FILELISTS_UPLOAD_STATS_ITEM_V2: return new RsFileListsUploadStatsItemV2();
default:
return NULL ;
}
Expand Down
20 changes: 20 additions & 0 deletions src/file_sharing/rsfilelistitems.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const uint8_t RS_PKT_SUBTYPE_FILELISTS_CONFIG_ITEM = 0x03;
const uint8_t RS_PKT_SUBTYPE_FILELISTS_BANNED_HASHES_ITEM = 0x04;
const uint8_t RS_PKT_SUBTYPE_FILELISTS_BANNED_HASHES_CONFIG_ITEM = 0x05;
const uint8_t RS_PKT_SUBTYPE_FILELISTS_UPLOAD_STATS_ITEM = 0x06;
const uint8_t RS_PKT_SUBTYPE_FILELISTS_UPLOAD_STATS_ITEM_V2 = 0x07;

/*!
* Base class for filelist sync items
Expand Down Expand Up @@ -139,6 +140,25 @@ class RsFileListsUploadStatsItem: public RsFileListsItem
std::map<RsFileHash, uint64_t> hash_stats;
};

struct TimeBasedUploadStat
{
uint64_t last_upload_ts;
Copy link
Contributor

@csoler csoler Feb 8, 2026

Choose a reason for hiding this comment

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

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.

uint64_t total_bytes;

bool operator==(const TimeBasedUploadStat& r) const { return last_upload_ts == r.last_upload_ts && total_bytes == r.total_bytes; }
};

class RsFileListsUploadStatsItemV2: public RsFileListsItem
{
public:
RsFileListsUploadStatsItemV2() : RsFileListsItem(RS_PKT_SUBTYPE_FILELISTS_UPLOAD_STATS_ITEM_V2){}

virtual void clear() { hash_stats.clear(); }
virtual void serial_process(RsGenericSerializer::SerializeJob j,RsGenericSerializer::SerializeContext& ctx);

std::map<RsFileHash, TimeBasedUploadStat> hash_stats;
};

class RsFileListsSerialiser : public RsServiceSerializer
{
public:
Expand Down
13 changes: 13 additions & 0 deletions src/ft/ftserver.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2363,3 +2363,16 @@ void ftServer::clearUploadStats()
{
return mFileDatabase->clearUploadStats();
}

void ftServer::setUploadStatsRetentionDays(int days)
{
if (mFileDatabase)
mFileDatabase->setUploadStatsRetentionDays(days);
}

int ftServer::getUploadStatsRetentionDays()
{
if (mFileDatabase)
return mFileDatabase->getUploadStatsRetentionDays();
return 0;
}
3 changes: 3 additions & 0 deletions src/ft/ftserver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ class ftServer :
virtual void setFilePermDirectDL(uint32_t perm) override;
virtual uint32_t filePermDirectDL() override;

virtual void setUploadStatsRetentionDays(int days) override;
virtual int getUploadStatsRetentionDays() override;

/// @see RsFiles
std::error_condition requestFiles(
const RsFileTree& collection,
Expand Down
14 changes: 13 additions & 1 deletion src/retroshare/rsfiles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,12 +792,24 @@ class RsFiles
virtual void setFilePermDirectDL(uint32_t perm)=0;

/**
* @brief Get Direct Download File Permission
* @brief Set Direct Download File Permission
* @jsonapi{development}
* @return mFilePermDirectDLPolicy direct download permission
*/
virtual uint32_t filePermDirectDL()=0;

/**
* @brief Set Upload Statistics Retention in Days
* @param days Number of days to keep upload statistics (0 = keep forever)
*/
virtual void setUploadStatsRetentionDays(int days) = 0;

/**
* @brief Get Upload Statistics Retention in Days
* @return Number of days to keep upload statistics
*/
virtual int getUploadStatsRetentionDays() = 0;

/**
* @brief Request remote files search
* @jsonapi{development}
Expand Down
2 changes: 2 additions & 0 deletions src/serialiser/rstlvkeys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ const uint32_t RSTLV_KEY_DISTRIB_ADMIN = 0x0040;
const uint32_t RSTLV_KEY_DISTRIB_IDENTITY = 0x0080;
const uint32_t RSTLV_KEY_DISTRIB_MASK = 0x00f0;

static const std::string UPLOAD_STATS_RETENTION_DAYS_SS = "UPLOAD_STATS_RETENTION_DAYS";

// Old class for RsTlvSecurityKey. Is kept for backward compatibility, but should not be serialised anymore

class RsTlvRSAKey: public RsTlvItem
Expand Down
Loading