[PATCH damus] storage: Improve clear cache functionality

[Daniel D’Aquino]

This patch improves clear cache functionality by:
- Reducing kingfisher cache removal to one command (The two commands running async was leading to warning logs. One was a subset of the other)
- Removing all files under the cache folder where not currently used by other processes
- Clearing up NostrDB

Functionality test
------------------

Device: iPhone 13 mini (Physical device)
iOS: 17.0.2
Special remarks:
- I had to disable some entitlements locally to push the local build to my physical phone.
- I had to locally delete other unit tests to be able to build the test target

Test steps:

1. Follow multiple active accounts
2. Scroll down on the feed for a couple of minutes (or until you have seen at least a few images, a few videos, and link previews)
3. In Xcode, download a storage container (Window > Devices and Simulators > Select the device > Select Damus > click on (...) > Download container)
- Note: Even though you see the file, it does not download instantly. Monitor the file size until it roughly reaches the size reported in iOS storage settings, as the download may still be in progress. This may take a few minutes in some cases.
4. Open the app data package using terminal
5. Run `du -h . | sort -hr`
6. Clear cache
7. Download a new storage container. Remember to wait until it completes download.
8. Run `du -h . | sort -hr` on it.
9. Compare. There should be very little data left after cache reset. Cache folder and Documents folder should be a lot smaller.
10. Go back to the home feed and start scrolling, browsing, follow some other people, etc. Look at your own profile as well. Everything should appear to be working as expected with no crashes or important data loss
11. Restart Damus and repeat the last step. Damus should be working as normal.
12. Start Damus with a debugger connection
13. Clear cache again while monitoring the debugger logs. There should be no dangerous looking messages.
14. Run `DamusCacheManagerTests`. Should pass

Results:
- Storage usage goes from 1.68 GB down to 4.4 MB.
- Damus works as normal after clearing cache, and after restarting the app as well. It becomes slower for a moment, and some user names are not visible for a few seconds, but after a bit it loads as normal again.
- No warning or error logs pertaining to clearing cache
- Unit test passes

Changelog-Changed: Improve clear cache functionality
Closes: https://github.com/damus-io/damus/issues/1472
Signed-off-by: Daniel D’Aquino <dan...@daquino.me>
---
Hi Will,

A few other notes about this:
- This patch builds on top of my patch for #1301 (cache clear UX feedback)
- Since we are dealing with essential components of the app, these changes bring some risk even though I tried to thoroughly test it. Please let me know if you identify any big risks with the code changes.

Thank you!
Daniel

damus.xcodeproj/project.pbxproj | 8 ++
damus/Models/DamusCacheManager.swift | 77 +++++++++++++++++++
damus/Util/Log.swift | 9 +++
damus/Views/ConfigView.swift | 20 -----
.../Settings/AppearanceSettingsView.swift | 2 +-
damusTests/DamusCacheManagerTests.swift | 31 ++++++++
nostrdb/Ndb.swift | 17 ++--
7 files changed, 137 insertions(+), 27 deletions(-)
create mode 100644 damus/Models/DamusCacheManager.swift
create mode 100644 damusTests/DamusCacheManagerTests.swift

diff --git a/damus.xcodeproj/project.pbxproj b/damus.xcodeproj/project.pbxproj
index 37aa8611..2c1e28f3 100644
--- a/damus.xcodeproj/project.pbxproj
+++ b/damus.xcodeproj/project.pbxproj
@@ -425,6 +425,8 @@
D723C38E2AB8D83400065664 /* ContentFilters.swift in Sources */ = {isa = PBXBuildFile; fileRef = D723C38D2AB8D83400065664 /* ContentFilters.swift */; };
D78525252A7B2EA4002FA637 /* NoteContentViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D78525242A7B2EA4002FA637 /* NoteContentViewTests.swift */; };
D7DEEF2F2A8C021E00E0C99F /* NostrEventTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */; };
+ D7315A2A2ACDF3B70036E30A /* DamusCacheManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7315A292ACDF3B70036E30A /* DamusCacheManager.swift */; };
+ D7315A2C2ACDF4DA0036E30A /* DamusCacheManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7315A2B2ACDF4DA0036E30A /* DamusCacheManagerTests.swift */; };
E4FA1C032A24BB7F00482697 /* SearchSettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4FA1C022A24BB7F00482697 /* SearchSettingsView.swift */; };
E990020F2955F837003BBC5A /* EditMetadataView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E990020E2955F837003BBC5A /* EditMetadataView.swift */; };
E9E4ED0B295867B900DD7078 /* ThreadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E9E4ED0A295867B900DD7078 /* ThreadView.swift */; };
@@ -1105,6 +1107,8 @@
D723C38D2AB8D83400065664 /* ContentFilters.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentFilters.swift; sourceTree = "<group>"; };
D78525242A7B2EA4002FA637 /* NoteContentViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoteContentViewTests.swift; sourceTree = "<group>"; };
D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NostrEventTests.swift; sourceTree = "<group>"; };
+ D7315A292ACDF3B70036E30A /* DamusCacheManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusCacheManager.swift; sourceTree = "<group>"; };
+ D7315A2B2ACDF4DA0036E30A /* DamusCacheManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusCacheManagerTests.swift; sourceTree = "<group>"; };
E4FA1C022A24BB7F00482697 /* SearchSettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchSettingsView.swift; sourceTree = "<group>"; };
E990020E2955F837003BBC5A /* EditMetadataView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditMetadataView.swift; sourceTree = "<group>"; };
E9E4ED0A295867B900DD7078 /* ThreadView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThreadView.swift; sourceTree = "<group>"; };
@@ -1288,6 +1292,7 @@
3A5E47C42A4A6CF400C0D090 /* Trie.swift */,
3A90B1802A4EA3AF00000D94 /* UserSearchCache.swift */,
D723C38D2AB8D83400065664 /* ContentFilters.swift */,
+ D7315A292ACDF3B70036E30A /* DamusCacheManager.swift */,
);
path = Models;
sourceTree = "<group>";
@@ -2148,6 +2153,7 @@
4C684A562A7FFAE6005E6031 /* UrlTests.swift */,
D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */,
D71DC1EB2A9129C3006E207C /* PostViewTests.swift */,
+ D7315A2B2ACDF4DA0036E30A /* DamusCacheManagerTests.swift */,
);
path = damusTests;
sourceTree = "<group>";
@@ -2653,6 +2659,7 @@
4C32B95C2A9AD44700DC3548 /* String+extension.swift in Sources */,
4C3BEFD22819DB9B00B3DE84 /* ProfileModel.swift in Sources */,
4CA352AA2A76BF3A003BB08B /* LocalNotificationNotify.swift in Sources */,
+ D7315A2A2ACDF3B70036E30A /* DamusCacheManager.swift in Sources */,
4C7D09682A0AE9B200943473 /* NWCScannerView.swift in Sources */,
4CA352A42A76AFF3003BB08B /* UpdateStatsNotify.swift in Sources */,
4C0A3F93280F66F5000448DE /* ReplyMap.swift in Sources */,
@@ -2906,6 +2913,7 @@
4C7D097E2A0C58B900943473 /* WalletConnectTests.swift in Sources */,
4CB883AA297612FF00DC99E7 /* ZapTests.swift in Sources */,
4CB8839A297322D200DC99E7 /* DMTests.swift in Sources */,
+ D7315A2C2ACDF4DA0036E30A /* DamusCacheManagerTests.swift in Sources */,
4C9054852A6AEAA000811EEC /* NdbTests.swift in Sources */,
75AD872B2AA23A460085EF2C /* Block+Tests.swift in Sources */,
F944F56E29EA9CCC0067B3BF /* DamusParseContentTests.swift in Sources */,
diff --git a/damus/Models/DamusCacheManager.swift b/damus/Models/DamusCacheManager.swift
new file mode 100644
index 00000000..9ff6a629
--- /dev/null
+++ b/damus/Models/DamusCacheManager.swift
@@ -0,0 +1,77 @@
+//
+// DamusCacheManager.swift
+// damus
+//
+// Created by Daniel D’Aquino on 2023-10-04.
+//
+
+import Foundation
+import Kingfisher
+
+struct DamusCacheManager { // We are using a struct just to organize functions within a common namespace
+
+ static func clear_cache(damus_state: DamusState, completion: (() -> Void)? = nil) {
+ Log.info("Clearing all caches", for: .storage)
+ clear_kingfisher_cache(completion: {
+ clear_cache_folder(completion: {
+ clear_nostrdb(damus_state: damus_state)
+ Log.info("All caches cleared", for: .storage)
+ completion?()
+ })
+ })
+ }
+
+ static func clear_kingfisher_cache(completion: (() -> Void)? = nil) {
+ Log.info("Clearing Kingfisher cache", for: .storage)
+ KingfisherManager.shared.cache.clearMemoryCache()
+ KingfisherManager.shared.cache.clearDiskCache {
+ Log.info("Kingfisher cache cleared", for: .storage)
+ completion?()
+ }
+ }
+
+ static func clear_cache_folder(completion: (() -> Void)? = nil) {
+ Log.info("Clearing entire cache folder", for: .storage)
+ let cacheURL = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
+
+ do {
+ let fileNames = try FileManager.default.contentsOfDirectory(atPath: cacheURL.path)
+
+ for fileName in fileNames {
+ let filePath = cacheURL.appendingPathComponent(fileName)
+
+ // Prevent issues by double-checking if files are in use, and do not delete them if they are.
+ // This is not perfect. There is still a small chance for a race condition if a file is opened between this check and the file removal.
+ let isBusy = (!(access(filePath.path, F_OK) == -1 && errno == ETXTBSY))
+ if isBusy {
+ continue
+ }
+
+ try FileManager.default.removeItem(at: filePath)
+ }
+
+ Log.info("Cache folder cleared successfully.", for: .storage)
+ completion?()
+ } catch {
+ Log.error("Could not clear cache folder", for: .storage)
+ }
+ }
+
+ static func clear_nostrdb(damus_state: DamusState) {
+ Log.info("Clearing NostrDB cache", for: .storage)
+
+ Log.debug("Stopping NostrDB threads to safely delete db file", for: .storage)
+ ndb_destroy(damus_state.ndb.ndb.ndb)
+
+ // Delete the database file
+ Log.debug("Deleting NostrDB db file", for: .storage)
+ let dataMdbPath = Ndb.db_path + "/data.mdb"
+ try? FileManager.default.removeItem(atPath: dataMdbPath)
+
+ // Restart the NostrDB threads
+ Log.debug("Restarting NostrDB", for: .storage)
+ damus_state.ndb.ndb = Ndb.start(path: Ndb.db_path)! // Not ideal to force unwrap, but if we cannot initialize NostrDB, we cannot recover.
+
+ Log.info("Successfully cleared NostrDB cache", for: .storage)
+ }
+}
diff --git a/damus/Util/Log.swift b/damus/Util/Log.swift
index fb753c0a..7811f90c 100644
--- a/damus/Util/Log.swift
+++ b/damus/Util/Log.swift
@@ -12,6 +12,7 @@ import os.log
enum LogCategory: String {
case nav
case render
+ case storage
}

/// Damus structured logger
@@ -44,4 +45,12 @@ class Log {
static func info(_ msg: StaticString, for logcat: LogCategory, _ args: CVarArg...) {
Log.log(msg, for: logger(for: logcat), type: OSLogType.info, args)
}
+
+ static func debug(_ msg: StaticString, for logcat: LogCategory, _ args: CVarArg...) {
+ Log.log(msg, for: logger(for: logcat), type: OSLogType.debug, args)
+ }
+
+ static func error(_ msg: StaticString, for logcat: LogCategory, _ args: CVarArg...) {
+ Log.log(msg, for: logger(for: logcat), type: OSLogType.error, args)
+ }
}
diff --git a/damus/Views/ConfigView.swift b/damus/Views/ConfigView.swift
index 4a51208a..76129b56 100644
--- a/damus/Views/ConfigView.swift
+++ b/damus/Views/ConfigView.swift
@@ -174,23 +174,3 @@ func handle_string_amount(new_value: String) -> Int? {

return amt
}
-
-func clear_kingfisher_cache(completion: (() -> Void)? = nil) {
- KingfisherManager.shared.cache.clearMemoryCache()
-
- let group = DispatchGroup()
-
- group.enter()
- KingfisherManager.shared.cache.clearDiskCache {
- group.leave()
- }
-
- group.enter()
- KingfisherManager.shared.cache.cleanExpiredDiskCache {
- group.leave()
- }
-
- group.notify(queue: .main) {
- completion?()
- }
-}
diff --git a/damus/Views/Settings/AppearanceSettingsView.swift b/damus/Views/Settings/AppearanceSettingsView.swift
index 59317632..3fdb3f1d 100644
--- a/damus/Views/Settings/AppearanceSettingsView.swift
+++ b/damus/Views/Settings/AppearanceSettingsView.swift
@@ -115,7 +115,7 @@ struct AppearanceSettingsView: View {
let group = DispatchGroup()

group.enter()
- clear_kingfisher_cache(completion: {
+ DamusCacheManager.clear_cache(damus_state: self.damus_state, completion: {
group.leave()
})

diff --git a/damusTests/DamusCacheManagerTests.swift b/damusTests/DamusCacheManagerTests.swift
new file mode 100644
index 00000000..8651fa06
--- /dev/null
+++ b/damusTests/DamusCacheManagerTests.swift
@@ -0,0 +1,31 @@
+//
+// DamusCacheManagerTests.swift
+// damusTests
+//
+// Created by Daniel D’Aquino on 2023-10-04.
+//
+
+import Foundation
+
+import Foundation
+import XCTest
+@testable import damus
+import SwiftUI
+
+final class DamusCacheManagerTests: XCTestCase {
+
+ override func setUpWithError() throws {
+ // Put setup code here. This method is called before the invocation of each test method in the class.
+ }
+
+ override func tearDownWithError() throws {
+ // Put teardown code here. This method is called after the invocation of each test method in the class.
+ }
+
+ /// Simple smoke test to check if clearing cache will crash the system
+ func testCacheManagerSmoke() throws {
+ for _ in Range(0...20) {
+ DamusCacheManager.clear_cache(damus_state: test_damus_state)
+ }
+ }
+}
diff --git a/nostrdb/Ndb.swift b/nostrdb/Ndb.swift
index e55c543e..5bfd32a0 100644
--- a/nostrdb/Ndb.swift
+++ b/nostrdb/Ndb.swift
@@ -8,7 +8,7 @@
import Foundation

class Ndb {
- let ndb: ndb_t
+ var ndb: ndb_t

static var db_path: String {
let path = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first?.absoluteString
@@ -23,6 +23,15 @@ class Ndb {
//try? FileManager.default.removeItem(atPath: Ndb.db_path + "/lock.mdb")
//try? FileManager.default.removeItem(atPath: Ndb.db_path + "/data.mdb")

+ guard let nostrdb_t = Ndb.start(path: path) else { return nil }
+ self.ndb = nostrdb_t
+ }
+
+ init(ndb: ndb_t) {
+ self.ndb = ndb
+ }
+
+ static func start(path: String? = nil) -> ndb_t? {
var ndb_p: OpaquePointer? = nil

let ingest_threads: Int32 = 4
@@ -45,11 +54,7 @@ class Ndb {
return nil
}

- self.ndb = ndb_t(ndb: ndb_p)
- }
-
- init(ndb: ndb_t) {
- self.ndb = ndb
+ return ndb_t(ndb: ndb_p)
}

func lookup_note_by_key_with_txn<Y>(_ key: NoteKey, txn: NdbTxn<Y>) -> NdbNote? {
--
2.39.1

[William Casarin]

On Wed, Oct 04, 2023 at 09:09:52PM +0000, Daniel D’Aquino wrote:
>This patch improves clear cache functionality by:
>- Reducing kingfisher cache removal to one command (The two commands running async was leading to warning logs. One was a subset of the other)
>- Removing all files under the cache folder where not currently used by other processes
>- Clearing up NostrDB

clear cache should not delete nostrdb. This would be very unintuitive
and nostrdb storage should be negligible. In the future we can have a
prune functionality that reduces the size of it.

[Daniel D'Aquino]

Sure, I can send a follow up patch without deleting nostrdb.

Would you prefer the nostrdb clear function to be in the patch but unused (to keep the work done in case we need it in the future), or should I delete the function entirely?

[William Casarin]

On Wed, Oct 04, 2023 at 09:30:50PM +0000, Daniel D'Aquino wrote:
>> clear cache should not delete nostrdb. This would be very unintuitive
>> and nostrdb storage should be negligible. In the future we can have a
>> prune functionality that reduces the size of it.
>

>Would you prefer the nostrdb clear function to be in the patch but
>unused (to keep the work done in case we need it in the future), or
>should I delete the function entirely?

There might be some scenarios where we would want to delete
everything... but only if the DB is critically large and needs to be
purged. I don't mind keeping the code.

[Daniel D'Aquino]

> On Oct 4, 2023, at 14:50, William Casarin <jb...@jb55.com> wrote:
>
> There might be some scenarios where we would want to delete
> everything... but only if the DB is critically large and needs to be
> purged. I don't mind keeping the code.

Sounds good, sent patch v2 for this!