Merge branch 'fix-contention' into 'master'

Try and fix contention issues

See merge request mangadex-pub/mangadex_at_home!101

build.gradle

@@ -6,7 +6,7 @@ plugins {
     id "application"
     id "com.github.johnrengelman.shadow" version "7.0.0"
     id "com.diffplug.spotless" version "5.8.2"
-    id "net.afanasev.sekret" version "0.1.1-RC3"
+    id "net.afanasev.sekret" version "0.1.1"
     id "com.palantir.git-version" version "0.12.3"
 }
 
@@ -28,7 +28,7 @@ configurations {
 dependencies {
     implementation "org.jetbrains.kotlin:kotlin-reflect"
 
-    compileOnly group: "net.afanasev", name: "sekret-annotation", version: "0.1.1-RC3"
+    compileOnly group: "net.afanasev", name: "sekret-annotation", version: "0.1.1"
 
     implementation group: "commons-io", name: "commons-io", version: "2.11.0"
     implementation group: "org.apache.commons", name: "commons-compress", version: "1.21"
@@ -66,7 +66,7 @@ dependencies {
 
     testImplementation "io.kotest:kotest-runner-junit5:$kotest_version"
     testImplementation "io.kotest:kotest-assertions-core:$kotest_version"
-    testImplementation "io.mockk:mockk:1.12.2"
+    testImplementation "io.mockk:mockk:1.12.3"
 }
 
 tasks.withType(Test) {

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.1.1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.1-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

src/main/kotlin/mdnet/Constants.kt

@@ -21,7 +21,7 @@ package mdnet
 import java.time.Duration
 
 object Constants {
-    const val CLIENT_BUILD = 31
+    const val CLIENT_BUILD = 32
 
     @JvmField val MAX_AGE_CACHE: Duration = Duration.ofDays(14)
 

src/main/kotlin/mdnet/cache/ImageStorage.kt

@@ -36,6 +36,7 @@ import java.sql.SQLException
 import java.time.Instant
 import java.util.UUID
 import java.util.concurrent.*
+import java.util.concurrent.locks.ReentrantLock
 
 @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class)
 data class ImageMetadata(
@@ -61,6 +62,7 @@ class ImageStorage(
     autoPrune: Boolean = true
 ) : AutoCloseable {
     private val tempCacheDirectory = cacheDirectory.resolve("tmp")
+    private val databaseLock = ReentrantLock()
 
     private val evictor: ScheduledExecutorService = Executors.newScheduledThreadPool(1)
     private val queue = LinkedBlockingQueue<String>(1000)
@@ -96,17 +98,24 @@ class ImageStorage(
                     val now = Instant.now()
 
                     LOGGER.info { "Updating LRU times for ${toUpdate.size} entries" }
-                    synchronized(database) {
+
-                        database.batchUpdate(DbImage) {
+                    if (databaseLock.tryLock(500, TimeUnit.MILLISECONDS)) {
-                            for (id in toUpdate) {
+                        try {
-                                item {
+                            database.batchUpdate(DbImage) {
-                                    set(DbImage.accessed, now)
+                                for (id in toUpdate) {
-                                    where {
+                                    item {
-                                        DbImage.id eq id
+                                        set(DbImage.accessed, now)
+                                        where {
+                                            DbImage.id eq id
+                                        }
                                     }
                                 }
                             }
+                        } finally {
+                            databaseLock.unlock()
                         }
+                    } else {
+                        LOGGER.warn { "High contention for database lock, bailing LRU update" }
                     }
                     calculateSize()
                 } catch (e: Exception) {
@@ -238,14 +247,24 @@ class ImageStorage(
             )
 
             Files.deleteIfExists(path)
-        } catch (e: IOException) {
+        } catch (_: IOException) {
-            // a failure means the image did not exist
+        }
-        } finally {
+
-            synchronized(database) {
+        // it is safe, but not optimal, for the
+        // DB write to fail after we've grabbed the file,
+        // as that just inflates the count.
+        // it will get resolved when the file gets grabbed again,
+        // or if the cache gets pruned.
+        if (databaseLock.tryLock(500, TimeUnit.MILLISECONDS)) {
+            try {
                 database.delete(DbImage) {
                     DbImage.id eq id
                 }
+            } finally {
+                databaseLock.unlock()
             }
+        } else {
+            LOGGER.warn { "High contention for database lock, bailing image delete write" }
         }
     }
 
@@ -319,23 +338,27 @@ class ImageStorage(
 
             Files.createDirectories(getPath(id).parent)
 
-            try {
+            if (databaseLock.tryLock(500, TimeUnit.MILLISECONDS)) {
-                synchronized(database) {
+                try {
                     database.insert(DbImage) {
                         set(DbImage.id, id)
                         set(DbImage.accessed, Instant.now())
                         set(DbImage.size, metadataSize + bytes)
                     }
+                } catch (e: SQLException) {
+                    // someone got to us before this (TOCTOU)
+                    // there are 2 situations here
+                    // one is that the
+                    // other write died in between writing the DB and
+                    // moving the file
+                    // the other is that we have raced and the other
+                    // is about to write the file
+                    // we handle this below
+                } finally {
+                    databaseLock.unlock()
                 }
-            } catch (e: SQLException) {
+            } else {
-                // someone got to us before this (TOCTOU)
+                LOGGER.warn { "High contention for database lock, bailing DB write" }
-                // there are 2 situations here
-                // one is that the
-                // other write died in between writing the DB and
-                // moving the file
-                // the other is that we have raced and the other
-                // is about to write the file
-                // we handle this below
             }
 
             try {