Merge pull request #978 from sirixdb/fix/hot-strict-binna-conformance

Fix/hot strict binna conformance

Author: JohannesLichtenberger

bundles/sirix-core/build.gradle

@@ -108,6 +108,21 @@ test {
     }
 
     jvmArgs = baseArgs
+    // The HOT soak test (hot.soak.run) sustains thousands of mutating revisions + concurrent
+    // readers; give its fork more heap. Normal test runs are unaffected.
+    if (System.getProperty('hot.soak.run') == 'true') {
+        maxHeapSize = System.getProperty('hot.soak.heap', '8g')
+        systemProperty 'hot.soak.run', 'true'
+        ['hot.soak.perRev', 'hot.soak.revs', 'hot.soak.readers', 'hot.soak.validateEvery',
+         'hot.soak.heap', 'hot.soak.seed', 'hot.soak.index',
+         'sirix.allocator.maxSize'].each { k -> if (System.getProperty(k) != null) systemProperty k, System.getProperty(k) }
+        // An OOM in a forked soak worker otherwise leaves the JVM hung indefinitely (the test
+        // never returns, gradle waits forever). Exit immediately and capture a heap dump so the
+        // run fails fast and the leak is diagnosable.
+        jvmArgs += ["-XX:+ExitOnOutOfMemoryError",
+                    "-XX:+HeapDumpOnOutOfMemoryError",
+                    "-XX:HeapDumpPath=${System.getenv('TMPDIR') ?: '/tmp'}/soak-oom.hprof"]
+    }
     // Pass through diagnostic system properties
     systemProperty "sirix.debug.path.summary", System.getProperty("sirix.debug.path.summary", "false")
     systemProperty "sirix.debug.leak.diagnostics", System.getProperty("sirix.debug.leak.diagnostics", "false")
@@ -121,7 +136,43 @@ test {
     systemProperty "sirix.lz4.fast.decompress", useFastLz4.toString()
     systemProperty "sirix.lz77Codec.native.disable", System.getProperty("sirix.lz77Codec.native.disable", "false")
     systemProperty "sirix.lz77Codec.diag.counters", System.getProperty("sirix.lz77Codec.diag.counters", "false")
-    
+    // HOT trie flags still referenced in source — forward from Gradle CLI to test JVM.
+    systemProperty "hot.strict.validate", System.getProperty("hot.strict.validate", "false")
+    systemProperty "hot.strict.phase7u.lexprimary", System.getProperty("hot.strict.phase7u.lexprimary", "false")
+    systemProperty "hot.strict.phase7u.lexfallback.disable", System.getProperty("hot.strict.phase7u.lexfallback.disable", "false")
+    systemProperty "hot.bisect.i8", System.getProperty("hot.bisect.i8", "false")
+    systemProperty "hot.bisect.from", System.getProperty("hot.bisect.from", "0")
+    systemProperty "hot.bisect.to", System.getProperty("hot.bisect.to", "600")
+    systemProperty "hot.dump.at", System.getProperty("hot.dump.at", "-1")
+    systemProperty "hot.debug.phase4", System.getProperty("hot.debug.phase4", "false")
+    systemProperty "hot.formal.verify", System.getProperty("hot.formal.verify", "false")
+    systemProperty "hot.strict.phase5e", System.getProperty("hot.strict.phase5e", "false")
+    systemProperty "hot.trace.persistent", System.getProperty("hot.trace.persistent", "false")
+    systemProperty "hot.strict.phase7f", System.getProperty("hot.strict.phase7f", "false")
+    systemProperty "hot.strict.phase7h", System.getProperty("hot.strict.phase7h", "false")
+    systemProperty "hot.strict.phase7j", System.getProperty("hot.strict.phase7j", "false")
+    systemProperty "hot.strict.phase7k", System.getProperty("hot.strict.phase7k", "false")
+    systemProperty "hot.strict.phase7q.commitroot", System.getProperty("hot.strict.phase7q.commitroot", "false")
+    systemProperty "hot.strict.phase7q.postcommit", System.getProperty("hot.strict.phase7q.postcommit", "false")
+    systemProperty "hot.strict.phase7r.routeverify", System.getProperty("hot.strict.phase7r.routeverify", "false")
+    systemProperty "hot.strict.phase7s.split", System.getProperty("hot.strict.phase7s.split", "true")
+    systemProperty "hot.strict.phase7t.monotone.probe", System.getProperty("hot.strict.phase7t.monotone.probe", "false")
+    systemProperty "hot.strict.phase7t.betamixed.probe", System.getProperty("hot.strict.phase7t.betamixed.probe", "false")
+    systemProperty "hot.strict.phase7t.crossroute.probe", System.getProperty("hot.strict.phase7t.crossroute.probe", "false")
+    systemProperty "hot.strict.phase7t.subsetroute.probe", System.getProperty("hot.strict.phase7t.subsetroute.probe", "false")
+    systemProperty "hot.strict.phase7t.perkey.probe", System.getProperty("hot.strict.phase7t.perkey.probe", "false")
+    systemProperty "hot.strict.phase7t10.perkey.probe", System.getProperty("hot.strict.phase7t10.perkey.probe", "false")
+    systemProperty "hot.strict.phase7t11.perkey.probe", System.getProperty("hot.strict.phase7t11.perkey.probe", "false")
+    systemProperty "hot.debug.i6trace", System.getProperty("hot.debug.i6trace", "")
+    systemProperty "hot.diag.selfHealDetail", System.getProperty("hot.diag.selfHealDetail", "false")
+    systemProperty "hot.diag.postHandlerValidate", System.getProperty("hot.diag.postHandlerValidate", "false")
+    systemProperty "hot.diag.directionOneFallback", System.getProperty("hot.diag.directionOneFallback", "false")
+    // Forward ALL hot.* system properties from the gradle JVM to the forked test JVM so that
+    // ad-hoc -Dhot.diag.* debug flags reach the code under test without needing to whitelist each
+    // one here (the individual entries above are kept for clarity / defaults but are redundant
+    // with this pattern). Mirrors the root build.gradle's sirix.* forwarding.
+    System.properties.findAll { it.key.startsWith("hot.") }.each { k, v -> systemProperty k, v }
+
     // Show test output for diagnostics
     testLogging {
         showStandardStreams = true

bundles/sirix-core/src/main/java/io/sirix/access/EmptyBufferManager.java

@@ -10,6 +10,7 @@
 import io.sirix.cache.RevisionRootPageCacheKey;
 import io.sirix.index.name.Names;
 import io.sirix.node.interfaces.Node;
+import io.sirix.page.HOTLeafPage;
 import io.sirix.page.KeyValueLeafPage;
 import io.sirix.page.PageReference;
 import io.sirix.page.RevisionRootPage;
@@ -23,6 +24,8 @@ public final class EmptyBufferManager implements BufferManager {
 
   private static final EmptyCache<PageReference, Page> PAGE_CACHE = new EmptyCache<>();
 
+  private static final EmptyCache<PageReference, HOTLeafPage> HOT_LEAF_PAGE_CACHE = new EmptyCache<>();
+
   private static final EmptyCache<RevisionRootPageCacheKey, RevisionRootPage> REVISION_ROOT_PAGE_CACHE =
       new EmptyCache<>();
 
@@ -49,6 +52,11 @@ public Cache<PageReference, Page> getPageCache() {
     return PAGE_CACHE;
   }
 
+  @Override
+  public Cache<PageReference, HOTLeafPage> getHOTLeafPageCache() {
+    return HOT_LEAF_PAGE_CACHE;
+  }
+
   @Override
   public Cache<RevisionRootPageCacheKey, RevisionRootPage> getRevisionRootPageCache() {
     return REVISION_ROOT_PAGE_CACHE;

bundles/sirix-core/src/main/java/io/sirix/access/trx/node/AbstractIndexController.java

@@ -236,6 +236,12 @@ public IndexController<R, W> createIndexListeners(final Set<IndexDef> indexDefs,
     return this;
   }
 
+  @Override
+  public void clearChangeListeners() {
+    listeners.clear();
+    primitiveListeners.clear();
+  }
+
   private void updateIndexCapability(final IndexType type) {
     switch (type) {
       case PATH -> hasPathIndex = true;

bundles/sirix-core/src/main/java/io/sirix/access/trx/node/IndexController.java

@@ -221,6 +221,15 @@ void notifyChange(ChangeType type, long nodeKey, NodeKind nodeKind, long pathNod
    */
   IndexController<R, W> createIndexListeners(Set<IndexDef> indexDefs, W nodeWriteTrx);
 
+  /**
+   * Remove all registered change listeners. A write-side controller may be cached and reused across
+   * write transactions; its listeners capture a specific transaction's storage engine and path
+   * summary, so a new transaction must clear the previous (now-closed) transaction's listeners
+   * before rebinding its own — otherwise {@link #notifyChange} could fire a listener against a
+   * closed transaction ("Transaction is already closed!").
+   */
+  void clearChangeListeners();
+
   NameFilter createNameFilter(Set<String> names);
 
   PathFilter createPathFilter(Set<String> paths, R rtx) throws PathException;

bundles/sirix-core/src/main/java/io/sirix/access/trx/node/json/JsonNodeTrxImpl.java

@@ -248,6 +248,10 @@ private boolean isFusedSyntheticChildCursor() {
 
     // Register index listeners for any existing indexes.
     // This is critical for subsequent write transactions to update indexes on node modifications.
+    // The write-side index controller is cached and reused across transactions, so first drop any
+    // listeners bound to a previous (now-closed) transaction before rebinding this one's — else a
+    // stale listener fires against a closed storage engine ("Transaction is already closed!").
+    indexController.clearChangeListeners();
     final var existingIndexDefs = indexController.getIndexes().getIndexDefs();
     if (!existingIndexDefs.isEmpty()) {
       indexController.createIndexListeners(existingIndexDefs, this);

bundles/sirix-core/src/main/java/io/sirix/access/trx/node/xml/XmlNodeTrxImpl.java

@@ -151,6 +151,10 @@ final class XmlNodeTrxImpl extends
 
     // Register index listeners for any existing indexes.
     // This is critical for subsequent write transactions to update indexes on node modifications.
+    // The write-side index controller is cached and reused across transactions, so first drop any
+    // listeners bound to a previous (now-closed) transaction before rebinding this one's — else a
+    // stale listener fires against a closed storage engine ("Transaction is already closed!").
+    indexController.clearChangeListeners();
     final var existingIndexDefs = indexController.getIndexes().getIndexDefs();
     if (!existingIndexDefs.isEmpty()) {
       indexController.createIndexListeners(existingIndexDefs, this);

bundles/sirix-core/src/main/java/io/sirix/access/trx/page/HOTRangeCursor.java

@@ -109,7 +109,6 @@ public byte[] valueBytes() {
   private HOTLeafPage currentLeaf;
   private int currentIndex;
   private boolean exhausted = false;
-  private boolean guardAcquired = false;
 
   // Pre-computed next entry (for hasNext/next pattern)
   private Entry nextEntry = null;
@@ -168,7 +167,6 @@ private void descendToFirstEntry() {
     }
 
     if (currentLeaf != null) {
-      acquireLeafGuard();
       advanceToValid();
     } else {
       exhausted = true;
@@ -229,42 +227,17 @@ private void advanceToValid() {
    * @return true if advanced to a new leaf, false if no more leaves
    */
   private boolean advanceToNextLeaf() {
-    // Release current leaf guard
-    releaseLeafGuard();
-
-    // Use reader's parent-based traversal
+    // reader.advanceToNextLeaf resolves the next leaf through HOTTrieReader.loadPage, which
+    // guards it as the reader's single guarded leaf (releasing the previous one). The cursor
+    // holds no guard of its own — currentLeaf stays live for as long as it is current.
     currentLeaf = reader.advanceToNextLeaf();
-
     if (currentLeaf == null) {
       return false;
     }
-
-    // Acquire guard on new leaf
-    acquireLeafGuard();
     currentIndex = 0;
     return true;
   }
 
-  /**
-   * Acquire guard on current leaf.
-   */
-  private void acquireLeafGuard() {
-    if (currentLeaf != null && !guardAcquired) {
-      currentLeaf.acquireGuard();
-      guardAcquired = true;
-    }
-  }
-
-  /**
-   * Release guard on current leaf.
-   */
-  private void releaseLeafGuard() {
-    if (currentLeaf != null && guardAcquired) {
-      currentLeaf.releaseGuard();
-      guardAcquired = false;
-    }
-  }
-
   @Override
   public boolean hasNext() {
     return positionedValid;
@@ -388,7 +361,8 @@ int getCurrentIndex() {
 
   @Override
   public void close() {
-    releaseLeafGuard();
+    // No guard to release here: HOTTrieReader.loadPage owns the single leaf guard and frees
+    // it on the next navigation or when the reader itself is closed.
     currentLeaf = null;
     nextEntry = null;
     positionedValid = false;