KnownFileList: bound known.met growth with per-hash cap + 30-day TTL - #598
Conversation
CKnownFileList demotes the existing m_knownFileMap[hash] entry to m_duplicateFileList whenever a new candidate hits the same MD4 but carries a different (name, date, size). No record ever leaves m_duplicateFileList, so on profiles where files are routinely touched (AV, indexers, backup tools, NTFS-via-Linux, SMB shares with coarse mtime granularity) the list grows without bound for the lifetime of the profile. Issue amule-project#597 reports a long-running install with 11,917 real shared files and 244,724 records on disk (~233 k duplicate-hash records), of which a single .nfo accumulated 1,257 historical variants. Add PruneDuplicates(), invoked from Save() before the count header is written. Per-hash cap is KNOWN_DUPLICATE_HASH_CAP=8; records sort by mtime descending and the K newest survive. Two protection sets that are honored regardless of the cap: * inUse: snapshot of CSharedFileList::m_Files_map at Save entry. FindKnownFile returns duplicate-list pointers to AddFile, which parks them in m_Files_map; deleting one would dangle the share-list pointer. CopyFileList runs outside CKnownFileList::list_mut to keep the existing CSharedFileList -> CKnownFileList lock order from SafeAddKFile intact (acquiring the same order here would set up a textbook ABBA). * m_pinnedDuplicates: a transient set of duplicate-list pointers that FindKnownFile / IsOnDuplicates returned during this session. This catches the dual-content-copy case (same hash in two shared paths) where AddFile rejects the second copy because the hash is already in m_Files_map -- without the pin the cap could drop the second copy's record and force a re-hash on next restart. Both sets are populated only after CSharedFileList::Reload runs at least once, so the prune is gated on m_initialShareScanComplete to avoid racing the first scan: an early Save (e.g. a hash task finishing before share-scan completes) would otherwise see empty sets and over-prune. The cap is per-hash, not global. The live m_knownFileMap entry is never touched -- it holds canonical state (download stats, AICH, kad keywords). Worst-case bound is unique_hashes * (1 + cap); on ngosang's profile that ceiling is ~108 k vs 244 k today.
The cap-only commit bounds m_duplicateFileList growth at unique_hashes * (1 + cap) and stops the multi-mtime-touch case (e.g. ngosang's .nfo files getting demoted on every backup-tool sweep) from accumulating without limit. But it does not address m_knownFileMap entries for hashes whose underlying file has not existed for years -- completed downloads that were since deleted, shares that were removed, files moved off this box. Those live entries are immortal under cap-only, and on a multi-year profile they make up the bulk of the bloat. Add a persisted "last seen alive" timestamp per CKnownFile, written as FT_LASTSEEN tag (uint32 epoch seconds, backward-compatible: older binaries skip unknown tags on load). Refreshed by CKnownFileList::FindKnownFile, IsOnDuplicates, the "already on the list" early-return in Append, and the freshly-hashed-record paths. PruneDuplicates extends to also walk m_knownFileMap: any record (live or duplicate) whose lastSeen is older than the TTL window AND isn't in inUse ∪ m_pinnedDuplicates is dropped. When a live entry expires the whole hash dies -- its duplicates are wiped too, otherwise they'd be orphaned references with no canonical record. Migration: a known.met written before FT_LASTSEEN was added leaves m_lastSeen at the Init() sentinel of 0. CKnownFile::LoadFromFile falls back to m_lastDateChanged (the file's stored mtime) in that case, so the first save after upgrade can actually evict ancient records rather than treating them as "fresh now" and waiting the full TTL window for them to age out. Subsequent saves use the genuine lastSeen written by previous matches. TTL is 30 days as a starting point. On ngosang's reported profile (244 k records, 11.9 k live shared files), the cap-only pass collapsed to roughly unique_hashes * 9 ~ 108 k worst case; with the TTL on top, hashes belonging to files that haven't been on disk recently get dropped outright, and the steady-state should hover near the live-shared count. The TTL prune runs only after MarkInitialShareScanComplete fires (same gate as the cap), so a hash-task-triggered Save before the first share-scan completes can't see empty in-use/pinned sets and over-prune.
| uint64 in_size) | ||
| { | ||
| wxMutexLocker sLock(list_mut); | ||
| const uint32 now = (uint32) time(NULL); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Good question, but this isn't a new Y2038 surface — the uint32 cast matches existing aMule convention. CKnownFile::m_lastDateChanged is time_t in memory but already gets persisted as uint32 via WriteUInt32 / ReadUInt32, as do FT_KADLASTPUBLISHSRC, FT_KADLASTPUBLISHNOTES, FT_ATTRANSFERRED, and the other time-valued tags in known.met / part.met. The new FT_LASTSEEN tag follows the same pattern; bumping it to 64-bit alone would be inconsistent and wouldn't help the records it has to compare against.
The Y2038 problem proper — signed 32-bit time_t overflowing to negative — is a platform property: if the OS's time_t is 32-bit signed, every aMule timestamp is affected, not just this patch. On modern 64-bit Linux / macOS / Windows that doesn't apply. The (uint32) time(NULL) cast on a 64-bit time_t keeps the low 32 bits, which represents the Unix epoch unsigned up to year 2106, not 2038 — so we actually get more headroom than a 32-bit signed time_t would.
A real codebase-wide Y2106 cleanup would have to touch the .met file formats. Worth its own issue if anyone's planning that far ahead, but seems out of scope for this PR.
There was a problem hiding this comment.
I agree with you, we are just following the same pattern as other time-valued tags in those files, and being unsigned this shall be ok until year 2106. Just for the sake of testing, I tried current master readiness for y2038, and there are some problems. I will open a separate issue for that: #602
|
Without going deep into the patch: What happens if amule is stopped for over 30 days? Will the patch cause a full rehash then? |
|
No, currently-shared files stay un-rehashed even after a multi-month outage. The order in a session is:
Only records the share-scan didn't touch — live entries whose file isn't in any shared dir anymore (deleted, removed from the list, moved off the box) and their duplicates — get TTL-evicted. Those wouldn't be re-hashed because there's no file to re-hash; their hash is just forgotten. The one case that does trigger a re-hash is mtime drift during the outage (something touched a shared file while amule was off), but that's existing aMule behavior — the patch doesn't change anything about when hashing kicks in. |
The previous commit's Append() unconditionally called SetLastSeen(now) in all four "this record is becoming the live entry" branches. The intent was to mark freshly-hashed records as just-seen-on-disk, so the TTL prune knows not to evict them. But Append() is also called from CKnownFileList::Init's load loop with afterHashing=false, and there the stamp does the opposite of what we want: every record loaded from known.met gets lastSeen overwritten with "now" as it passes through Append, including records that the demote branch then immediately pushes onto m_duplicateFileList. The end state after load is every duplicate appearing fresh, which makes the migration fallback (LoadFromFile -> "lastSeen = m_lastDateChanged when no FT_LASTSEEN tag present") a no-op and the TTL pass never evicts anything. Reported on the PR amule-project#598 thread by @ngosang -- his test pruned 209,127 duplicate records by cap but dropped 0 by TTL on a 244 k profile that should have lost most of its 232 k duplicates to TTL on first save (mtimes years old). Gate the four Append SetLastSeen calls on `afterHashing`. The parameter already exists and is set true by SafeAddKFile post-hash callers and false by the load loop. So: * Load path (afterHashing=false): preserves whatever LoadFromFile set, which is either a real FT_LASTSEEN tag value or the m_lastDateChanged migration fallback. TTL prune now has real signal to work with. * Post-hash path (afterHashing=true): stamps lastSeen=now as before, so fresh hashes aren't born aged-out. Share-scan path is unchanged -- FindKnownFile does its own SetLastSeen(now) on every match, so currently-shared files still get their live entry refreshed regardless of how Append behaves. The "on duplicates list" branch's pin into m_pinnedDuplicates is also gated on afterHashing: pinning during load would falsely protect stale records from the cap/TTL prune. FindKnownFile's pin during share-scan is the legitimate path. Expected effect on ngosang's profile after this fix: the 35 k post-cap residue should drop further toward ~live + recently-active duplicates, because the ~23 k duplicates that survived the cap have mtimes years old and the TTL filter will now see them.
known2_64.met is the AICH (Advanced Intelligent Corruption Handling) hashset cache: a Merkle tree of SHA-1 hashes per shared file, used by peers to recover from sub-part-granularity corruption. Each entry is keyed by its AICH root hash; ngosang's #597 report had it at 1.1 GB. The file has no mtime-touch bloat (entries are content-addressed, dedup is already in place since #581), but it never shrinks: once a hashset is cached, the entry stays even after the underlying file leaves the user's library. After #598's known.met TTL prune drops ~14 k orphaned live entries on a long-lived profile, those hashes' AICH entries in known2_64.met are dead weight and should follow them out. Add CKnownFileList::CollectLiveAICHRoots() -- walks m_knownFileMap and m_duplicateFileList under list_mut, returns the set of AICH master hashes still referenced by either. Both lists need to be scanned: Append's demote branch parks a record (with its hashset) on the duplicate list while the new record takes the live slot, and an mtime-restore can re-promote the duplicate later. Dropping a duplicate's hashset would silently lose it on re-promote. Extend CAICHSyncTask::Entry()'s existing known2_64.met walk: open a "<name>.new" temp file via CFile::write_safe, and for each entry read from the source, either copy it through to the temp (if its root hash is in liveRoots) or skip it. On clean walk completion the Close() atomic-renames .new over the original; on corruption catch or IO error the temp is removed without finalising, leaving the source's existing truncation-recovery path intact. Hashset bytes are streamed through a 64 KB buffer rather than slurped, so a single large-file entry can't dominate the working set. The dedup root-hash cache (s_rootHashCache, #581) mirrored the old file; invalidate it after a non-zero drop so the next SaveHashSet rebuilds against the rewritten known2_64.met. Effective TTL is inherited from known.met: a record evicted there by PruneDuplicates ages out of liveRoots and its hashset gets dropped on the next AICH sync. Decoupled lifecycles would require bumping KNOWN2_MET_VERSION to add a per-entry timestamp (the file format is positional, not tag-based), which is out of scope here. Defensive: if knownfiles isn't yet populated (empty liveRoots), the prune is skipped -- we don't wipe everything on a misconfigured start.
…rop dead VBT path (amule-project#598) Sizes the ed2k block-transfer pipeline to the bandwidth-delay product (BDP) in both directions, so throughput holds up on low-latency LAN links and high-latency internet links alike, and removes the dead VBT (value-based-type-tags) ed2k-v2 code path. Download - adaptive request depth. Instead of a fixed 3-deep request queue, each source's depth is sized to its BDP, 2*(rate*minRTT/EMBLOCKSIZE)+3, clamped to [3,24]. It stays shallow against a fast LAN peer (avoiding the burst/starve oscillation a deep queue provokes on a sub-millisecond link) and goes deep against a high-latency WAN peer (hiding the round-trip). minRTT is measured per source as the minimum request->first-byte time; the wire still carries the protocol-mandated 3 blocks per OP_REQUESTPARTS. The 24 clamp is deliberate: socket telemetry showed the WAN ceiling is the OS TCP send-buffer autotune limit (~4 MB -> ~40 MB/s at 100 ms RTT), which 24 (~4.3 MB in flight) already covers; 24 also stays within eMule's own pending range (gate 2*blockCount = 18, with a top-up batch reaching ~27). Upload - adaptive send-ahead depth. Raises the disk I/O thread's fast-slot send-ahead buffer from 5 to 10 blocks: the upload-side mirror of the same BDP limit. A shallow buffer drains before the next refill and caps throughput on a high-RTT link. Cost is a transient ~1.8 MB per active fast slot, self-bounded by the OS TCP send buffer. VBT removal. Drops the value-based-type-tags ed2k-v2 path, which never shipped in a release (nValueBasedTypeTags was hardcoded 0 on send and GetVBTTags() always returned false), simplifying the block-request and known-file packet builders. Benchmarks (macOS leecher <-> bare-metal Linux seeder, dummynet-injected latency, 120 s steady state, current master vs this PR, both sides matched per config): RTT Stock (fixed-3 + buf-5) This PR (cap-24 + buf-10) 1 ms 131 MB/s 128 MB/s (no regression) 50 ms 5.8 MB/s 65 MB/s (~11x) 100 ms 3.0 MB/s 35 MB/s (~12x) The WAN ceiling is the OS TCP send buffer, not request depth (verified via ss -tmi); raising net.ipv4.tcp_wmem is host tuning, out of scope.
Summary
Addresses #597 —
known.metgrowing without bound over the lifetime of a profile. On the reporter's install, a multi-year-old profile shows 244,724 records for 11,917 currently-shared files, with a single .nfo file accumulating 1,257 historical mtime variants.Two coordinated mechanisms, one per commit:
Per-hash cap (commit 1) —
m_duplicateFileListretains at most 8 historical (name, date, size) variants per hash, newest mtime survives. Bounds within-hash bloat (the .nfo touch pattern). Livem_knownFileMapentries never touched. Two protection sets honored regardless of cap: pointers currently inCSharedFileList::m_Files_map(deleting one would dangle the share-list pointer) and a session-local pin set populated whenFindKnownFilereturns a duplicate-list pointer (catches the dual-content-copy-in-shared-dirs case where the duplicate is real on-disk butAddFilerejected it as a content duplicate).30-day TTL (commit 2) — Adds persisted
FT_LASTSEENtag (uint32 epoch, backward-compatible — older binaries ignore unknown tags). Refreshed on everyFindKnownFile/IsOnDuplicates/ "already on the list" match. Records past the TTL window get dropped: live entries cause their whole hash (including any duplicates) to be wiped, since a non-refreshed live entry means no share-scan in 30 days produced a(name, date, size)match — the file isn't accessible. Migration: known.met records written before this tag existed default tom_lastDateChangedas the lastSeen fallback, so the first save after upgrade can actually evict ancient records rather than treating everything as "fresh now" for the next 30 days.Both passes are gated on
MarkInitialShareScanComplete, set byCSharedFileList::Reloadat the end of a successful share-scan. This prevents a hash-task-triggeredSave()running before the first share-scan from seeing empty protection sets and over-pruning.Lock-order safety: the in-use snapshot is taken via
CSharedFileList::CopyFileListbefore acquiringCKnownFileList::list_mut, preserving the existing SharedFileList→KnownFileList lock order used bySafeAddKFile(acquiring KnownFileList→SharedFileList here would be a textbook ABBA).Kept as draft while @ngosang validates the eviction behavior against his reported 244 k profile.