From f15dd06473f5fb82bb8e95b7d1523a9d74cdd61a Mon Sep 17 00:00:00 2001
From: Viktor Lofgren <vlofgren@marginalia.nu>
Date: Tue, 23 Jan 2024 11:08:41 +0100
Subject: [PATCH] (index) Delayed close() of SearchIndexReader

This avoids concurrent access errors.  This is especially important when using Unsafe-based LongArrays, since we have concurrent access to the underlying memory-mapped file.  If pull the rug from under the caller by closing the file, we'll get a SIGSEGV.  Even with a "safe" MemorySegment, we'll get ugly stacktraces if we close the file while a thread is still accessing it.

So we spin up a thread that sleeps for a minute before actually unmapping the file, allowing any ongoing requests to wrap up.  This is 100% a hack, but it lets us get away with doing this without adding locks to the index readers.

Since this is "just" mmapped data, and this operation happens optimistically once a month, it should be safe if the call gets lost.
---
 .../index/index/SearchIndexReader.java        | 40 ++++++++++---------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/code/services-core/index-service/src/main/java/nu/marginalia/index/index/SearchIndexReader.java b/code/services-core/index-service/src/main/java/nu/marginalia/index/index/SearchIndexReader.java
index 4d76fc09..bef389b0 100644
--- a/code/services-core/index-service/src/main/java/nu/marginalia/index/index/SearchIndexReader.java
+++ b/code/services-core/index-service/src/main/java/nu/marginalia/index/index/SearchIndexReader.java
@@ -5,9 +5,11 @@ import nu.marginalia.index.forward.ForwardIndexReader;
 import nu.marginalia.index.forward.ParamMatchingQueryFilter;
 import nu.marginalia.index.query.*;
 import nu.marginalia.index.query.filter.QueryFilterStepIf;
+import nu.marginalia.model.EdgeUrl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.time.Duration;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -69,29 +71,29 @@ public class SearchIndexReader {
     }
 
     public void close() throws InterruptedException {
-        tryClose(forwardIndexReader::close);
-        tryClose(reverseIndexFullReader::close);
-        tryClose(reverseIndexPriorityReader::close);
+       /* Delay the invocation of close method to allow for a clean shutdown of the service.
+        *
+        * This is especially important when using Unsafe-based LongArrays, since we have
+        * concurrent access to the underlying memory-mapped file.  If pull the rug from
+        * under the caller by closing the file, we'll get a SIGSEGV.  Even with MemorySegment,
+        * we'll get ugly stacktraces if we close the file while a thread is still accessing it.
+        */
+
+        delayedCall(forwardIndexReader::close, Duration.ofMinutes(1));
+        delayedCall(reverseIndexFullReader::close, Duration.ofMinutes(1));
+        delayedCall(reverseIndexPriorityReader::close, Duration.ofMinutes(1));
     }
 
-    /* Try to close the given resource, retrying a few times if it fails.
-     *  There is a small but non-zero chance we're closing during a query,
-     *  which will cause an IllegalStateException to be thrown.  We don't
-     *  want to add synchronization to the query code, so we just retry a
-     *  few times, fingers crossed.  If worse comes to worst, and we leak resources,
-     *  the GC will clean this up eventually...
-     * */
-    private void tryClose(Runnable closeMethod) throws InterruptedException {
-        for (int i = 0; i < 5; i++) {
+
+    private void delayedCall(Runnable call, Duration delay) throws InterruptedException {
+        Thread.ofPlatform().start(() -> {
             try {
-                closeMethod.run();
-                return;
-            } catch (Exception ex) {
-                logger.error("Error closing", ex);
+                TimeUnit.SECONDS.sleep(delay.toSeconds());
+                call.run();
+            } catch (InterruptedException e) {
+                logger.error("Interrupted", e);
             }
-            TimeUnit.MILLISECONDS.sleep(50);
-        }
-        logger.error("Failed to close index");
+        });
     }
 
     /** Returns true if index data is available */