ENGKNOW-3657: Reduce Table Service dictionary memory (tunable TableCache + lazy content map)

docs/superpowers/runbooks/2026-07-08-engknow-3657-profiling-runbook.md

@@ -0,0 +1,43 @@
+# ENGKNOW-3657 Profiling Runbook
+
+## 1. Get the prod heap ceiling
+Fetch the table-service pod memory limit from its helm values (or Grafana
+`kube_pod_container_resource_limits{resource="memory"}`). Use it as `-Xmx` below
+so the local run reproduces the OOM.
+
+## 2. Build the classpath
+```bash
+./gradlew :test:testClasses
+CP=$(./gradlew -q :test:printTestClasspath 2>/dev/null || echo "see note")
+# If :printTestClasspath is not defined, run via gradle JavaExec or use the
+# test runtime classpath from `./gradlew :test:dependencies`.
+```
+
+## 3. Run under JFR + heap-dump-on-OOM at the prod ceiling
+```bash
+java -Xmx<PROD_LIMIT> \
+     -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp/engknow3657.hprof \
+     -XX:StartFlightRecording=filename=/tmp/engknow3657.jfr,settings=profile,dumponexit=true \
+     -Dgor.memtest.fileCount=20000 -Dgor.memtest.tableCount=16 \
+     -Dgor.memtest.threads=8 -Dgor.memtest.readWritePercent=70 \
+     -Dgor.memtest.durationSeconds=300 \
+     -cp "$CP" org.gorpipe.test.memory.TableServiceLoadMain
+```
+Ramp `fileCount` / `tableCount` until it OOMs at the prod ceiling.
+
+## 4. Analyze
+- Heap dump: open `engknow3657.hprof` in Eclipse MAT or `jhat`. Sort by
+  **retained size**. Run "Dominator Tree" and "Path to GC Roots" on the top
+  objects. Check the hypotheses: `DictionaryEntries.rawLines` /
+  `tagHashToLines` / `contentHashToLines`; `byte[]` part buffers (~64 MB);
+  cache maps.
+- JFR: open `engknow3657.jfr` in JDK Mission Control → Memory → Allocation.
+  Identify top allocation sites.
+- Off-heap: compare `peakRssMB` vs `peakHeapMB` from the harness summary. A
+  large gap points at native/CRT S3 buffers — rerun with `-XX:NativeMemoryTracking=summary`
+  and `jcmd <pid> VM.native_memory summary`.
+
+## 5. Fallback: prod heap dump
+If it will not reproduce locally, add `-XX:+HeapDumpOnOutOfMemoryError`
+`-XX:HeapDumpPath=/data` to the table-service pod JVM args and pull the dump
+after the next OOM.

model/build.gradle

@@ -37,7 +37,9 @@ project(':model') {
         annotationProcessor 'com.google.auto.service:auto-service:_'
 
         implementation 'com.google.auto.service:auto-service:_'
-        implementation 'com.github.ben-manes.caffeine:caffeine:_'
+        // TableCache exposes CacheStats (com.github.benmanes.caffeine.cache.stats) on its public API,
+        // so callers need caffeine at compile time too.
+        api 'com.github.ben-manes.caffeine:caffeine:_'
         implementation "org.apache.commons:commons-lang3:_"
         implementation "org.aeonbits.owner:owner:_"
         implementation "com.github.samtools:htsjdk:_"

model/src/main/java/org/gorpipe/gor/table/TableCache.java

@@ -2,6 +2,7 @@
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.stats.CacheStats;
 import org.gorpipe.gor.model.FileReader;
 import org.gorpipe.util.Strings;
 import org.slf4j.Logger;
@@ -18,8 +19,10 @@ public abstract class TableCache<T extends Table> {
     public static final boolean useCache = Boolean.parseBoolean(System.getProperty("gor.dictionary.cache.active", "true"));
 
     final protected Cache<String, T> dictCache = Caffeine.newBuilder()
-            .maximumSize(500).expireAfterAccess(Duration.ofHours(12L))
+            .maximumSize(Long.getLong("gor.dictionary.cache.maxsize", 64L))
+            .expireAfterAccess(Duration.ofHours(Long.getLong("gor.dictionary.cache.ttl.hours", 12L)))
             .softValues()
+            .recordStats()
             .build();   //A map from dictionaries to the cache objects.
 
     abstract protected T createTable(String path, FileReader fileReader);
@@ -60,6 +63,18 @@ public synchronized T getTable(String path, FileReader fileReader, boolean useCa
         }
     }
 
+    public CacheStats stats() {
+        return dictCache.stats();
+    }
+
+    public long estimatedSize() {
+        return dictCache.estimatedSize();
+    }
+
+    public void cleanUp() {
+        dictCache.cleanUp();
+    }
+
     public synchronized void updateCache(T table) {
         String uniqueID = table.getId();
         var key = dictCacheKeyFromPathAndRoot(table.getPath(), table.getFileReader());

model/src/main/java/org/gorpipe/gor/table/dictionary/DictionaryEntries.java

@@ -37,25 +37,35 @@
 /**
  * Class that handles the loading caching of and working with table entries.
  *
- * Class is thread safe.
+ * <p>Thread-safety: safe for concurrent <b>reads</b> on a shared instance, including
+ * concurrent lazy first-load. Loaders and mutators ({@code insert}/{@code delete}/
+ * {@code clear}/{@code loadLinesAndUpdateIndices}/{@code updateTagMap}/{@code updateContentMap})
+ * are {@code synchronized}, and the lazily-published state fields are {@code volatile} so the
+ * unsynchronized read accessors observe fully-constructed data. Mutation must be externally
+ * serialized and must NOT run concurrently with reads on the same instance (structural
+ * modification of the backing list would break in-flight iteration); share a read-only instance
+ * for concurrent reads and use a separate instance for writes.
  */
 public class DictionaryEntries<T extends DictionaryEntry> implements IDictionaryEntries<T> {
     private static final Logger log = LoggerFactory.getLogger(DictionaryEntries.class);
     private final IDictionaryEntryFactory<T> factory;
 
-    private List<T> rawLines;
+    // volatile: lazily built under synchronized builders, but read by unsynchronized accessors
+    // (e.g. getEntries()/isLoaded()); volatile gives those reads safe publication.
+    private volatile List<T> rawLines;
 
     // For indices we use hashed values.  Insert into the dict is much much faster, and it takes a lot less space.  Getting data takes
     // a little bit longer as you could get small list of values you need to loop through.
-    private ListMultimap<Integer, T> tagHashToLines;  // tags here means aliases and tags.
-    private ListMultimap<Integer, T> contentHashToLines;
+    private volatile ListMultimap<Integer, T> tagHashToLines;  // tags here means aliases and tags.
+    private volatile ListMultimap<Integer, T> contentHashToLines;
 
-    private Multiset<String> activeTags;
-    private int deletedEntriesCount = 0;
+    private volatile Multiset<String> activeTags;
+    private int deletedEntriesCount = 0;  // only mutated/read under (or after) a synchronized builder.
     private final TableInfo table;
 
-    private boolean dataLoaded = false;
-    private boolean tagHashLoaded = false;
+    private volatile boolean dataLoaded = false;
+    private volatile boolean tagHashLoaded = false;
+    private volatile boolean contentMapLoaded = false;
 
     /**
      * Construct new dict file from the given path and chromosome cache.
@@ -152,9 +162,7 @@ public Iterator<T> iterator() {
 
     @Override
     public Iterator<T> getActiveEntries() {
-        if (!dataLoaded) {
-            loadLinesAndUpdateIndices();
-        }
+        updateContentMap();
         return new Iterator<T>() {
 
             int nextIndex = 0;
@@ -186,14 +194,13 @@ public boolean isLoaded() {
 
     @Override
     public Set<String> getAllActiveTags() {
-        if (!dataLoaded) {
-            loadLinesAndUpdateIndices();
-        }
+        updateContentMap();
         return activeTags.elementSet();
     }
 
     @Override
     public boolean hasDeletedEntries() {
+        updateContentMap();
         return deletedEntriesCount > 0;
     }
 
@@ -204,18 +211,24 @@ public int size() {
 
     @Override
     public int getActiveLinesCount() {
+        updateContentMap();
         int size = getEntries().size();
         return size - deletedEntriesCount;
     }
 
 
-    private void updateContentMap() {
+    synchronized private void updateContentMap() {
+        if (contentMapLoaded) return;
+        if (!dataLoaded) {
+            loadLinesAndUpdateIndices();
+        }
         contentHashToLines = ArrayListMultimap.create(rawLines.size(), 1);
         activeTags = HashMultiset.create();
-
+        deletedEntriesCount = 0;
         for (T entry : rawLines) {
             addEntryToContentMap(entry);
         }
+        contentMapLoaded = true;
     }
 
     synchronized private void updateTagMap() {
@@ -274,6 +287,7 @@ private void clearContentMap() {
         contentHashToLines = null;
         activeTags = null;
         deletedEntriesCount = 0;
+        contentMapLoaded = false;
     }
 
     private void clearTagMap() {
@@ -285,7 +299,6 @@ synchronized private void loadLinesAndUpdateIndices() {
         if (dataLoaded) return;
         log.trace("Loading entries into table {} {}", table.getName(), table);
         this.rawLines = this.loadLines();
-        this.updateContentMap();
         if (table.getColumns().length == 0 && !this.rawLines.isEmpty()) {
             try {
                 // Update header from the first file.
@@ -338,6 +351,7 @@ private List<T> loadLines() {
     public T findLine(T line) {
         // Using contentHashMap
         List<T> allLines = getEntries();
+        updateContentMap();
         List<T> lines2Search = contentHashToLines != null ? contentHashToLines.get(line.getSearchHash()) : allLines;
 
         return lines2Search != null ? lines2Search.stream().filter(l -> l.equals(line)).findFirst().orElse(null) : null;

model/src/main/java/org/gorpipe/gor/table/dictionary/DictionaryTableReader.java

@@ -51,11 +51,13 @@ public class DictionaryTableReader<T extends DictionaryEntry> extends TableInfoB
 
     private static final Logger log = LoggerFactory.getLogger(DictionaryTableReader.class);
 
-    private String contentType = null;  // For caching content type.
+    private volatile String contentType = null;  // For caching content type. volatile: lazily computed in inferShouldBucketizeFromContent() without a lock.
 
     protected IDictionaryEntries<T> tableEntries;
 
-    protected DictionaryAccessOptimizer tableAccessOptimizer;
+    // volatile: lazily built in getTableAccessOptimizer() (double-checked) and cleared in reload();
+    // volatile gives safe publication to concurrent readers.
+    protected volatile DictionaryAccessOptimizer tableAccessOptimizer;
 
     protected IDictionaryEntryFactory<T> factory;
 
@@ -156,10 +158,15 @@ public List<T> needsBucketizing() {
     }
 
     public Boolean inferShouldBucketizeFromContent() {
-        if (contentType == null) {
-            contentType = getFileEndingFromContent();
+        // Read the volatile field once into a local; the computation is idempotent, so a race
+        // between concurrent callers just recomputes the same value (last write wins) rather
+        // than needing a lock. Using the local avoids returning a racily-published field read.
+        String ct = contentType;
+        if (ct == null) {
+            ct = getFileEndingFromContent();
+            contentType = ct;
         }
-        return inferShouldBucketizeFromType(contentType);
+        return inferShouldBucketizeFromType(ct);
     }
 
     public Boolean inferShouldBucketizeFromFile(String fileName) {
@@ -322,10 +329,19 @@ public List<T> getOptimizedLines(Set<String> tags, boolean allowBucketAccess, bo
     }
 
     protected DictionaryAccessOptimizer getTableAccessOptimizer() {
-        if (tableAccessOptimizer == null) {
-            tableAccessOptimizer = new DefaultTableAccessOptimizer(this, tableEntries);
+        // Double-checked locking on the volatile field: concurrent readers get a single,
+        // safely-published optimizer instead of racing to build separate ones.
+        DictionaryAccessOptimizer optimizer = tableAccessOptimizer;
+        if (optimizer == null) {
+            synchronized (this) {
+                optimizer = tableAccessOptimizer;
+                if (optimizer == null) {
+                    optimizer = new DefaultTableAccessOptimizer(this, tableEntries);
+                    tableAccessOptimizer = optimizer;
+                }
+            }
         }
-        return tableAccessOptimizer;
+        return optimizer;
     }
 
 

model/src/main/java/org/gorpipe/gor/table/livecycle/TableInfoBase.java

@@ -47,7 +47,7 @@ public abstract class TableInfoBase implements TableInfo {
     private final String linkPath;      // Path to the link file.
     private final String rootUri;       // uri to table root (just to improve performance when working with uri's).
     private String name;          // Name of the table.
-    protected String id = null;         // Unique id (based on full path (and possibly timestamp), just so we don't always have to refer to full path).
+    protected volatile String id = null;         // Unique id (based on full path (and possibly timestamp), just so we don't always have to refer to full path).  volatile: lazily computed in getId() without synchronization.
 
     protected TableHeader header; // Header info.
 

model/src/test/java/org/gorpipe/gor/table/UTestTableCache.java

@@ -0,0 +1,57 @@
+package org.gorpipe.gor.table;
+
+import org.gorpipe.gor.model.DriverBackedFileReader;
+import org.gorpipe.gor.table.dictionary.gor.GorDictionaryCache;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class UTestTableCache {
+
+    @Rule
+    public TemporaryFolder workDir = new TemporaryFolder();
+
+    private Path writeSmallDict(String name) throws Exception {
+        Path dir = workDir.getRoot().toPath();
+        Files.writeString(dir.resolve("a.gor"), "chrom\tpos\n" + "chr1\t1\n");
+        Path gord = dir.resolve(name);
+        // one data line: file<TAB>alias
+        Files.writeString(gord, "a.gor\tpn1\n");
+        return gord;
+    }
+
+    @Test
+    public void getTable_secondCallIsCacheHit_andStatsRecorded() throws Exception {
+        Path gord = writeSmallDict("stats.gord");
+        GorDictionaryCache cache = new GorDictionaryCache();
+        DriverBackedFileReader reader = new DriverBackedFileReader("");
+
+        long hitsBefore = cache.stats().hitCount();
+        cache.getTable(gord.toString(), reader);   // miss -> put
+        cache.getTable(gord.toString(), reader);   // hit
+        Assert.assertEquals(1L, cache.estimatedSize());
+        Assert.assertTrue("expected a cache hit to be recorded",
+                cache.stats().hitCount() > hitsBefore);
+    }
+
+    @Test
+    public void maxSizeSystemProperty_isHonored() throws Exception {
+        String prev = System.getProperty("gor.dictionary.cache.maxsize");
+        System.setProperty("gor.dictionary.cache.maxsize", "1");
+        try {
+            GorDictionaryCache cache = new GorDictionaryCache();
+            DriverBackedFileReader reader = new DriverBackedFileReader("");
+            cache.getTable(writeSmallDict("m1.gord").toString(), reader);
+            cache.getTable(writeSmallDict("m2.gord").toString(), reader);
+            cache.cleanUp();
+            Assert.assertTrue("cache bounded to maxsize=1", cache.estimatedSize() <= 1L);
+        } finally {
+            if (prev == null) System.clearProperty("gor.dictionary.cache.maxsize");
+            else System.setProperty("gor.dictionary.cache.maxsize", prev);
+        }
+    }
+}