Merge pull request #874 from sirixdb/refactor/rename-resource-manager-to-session

refactor: miscellaneous cleanup across core pages and storage engine

Author: JohannesLichtenberger

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

@@ -351,8 +351,9 @@ public List<Path> listResources() {
 
   @Override
   public Transaction beginTransaction() {
-    // FIXME
-    return null;
+    throw new UnsupportedOperationException(
+        "Multi-resource transactions are not yet implemented. "
+            + "Use beginNodeTrx() on individual ResourceSession instances instead.");
   }
 
   @Override

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

@@ -2365,9 +2365,9 @@ private void copy(final JsonNodeReadOnlyTrx trx, final InsertPosition insert) {
         }
       }
       // $CASES-OMITTED$
-      default -> // new JsonItemShredder.Builder(this, new JsonItemIterator(new JsonItemFactory().get),
-                 // insert).build().call();
-        throw new IllegalStateException(); // FIXME
+      default -> throw new UnsupportedOperationException(
+          "Copying complex JSON node kinds (OBJECT, ARRAY, OBJECT_KEY) via copySubtree* "
+              + "is not yet implemented. Node kind: " + kind);
     }
     rtx.close();
   }

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

@@ -800,32 +800,43 @@ public UberPage commit(@Nullable final String commitMessage, @Nullable final Ins
       setUserIfPresent();
       setCommitMessageAndTimestampIfRequired(commitMessage, commitTimestamp);
 
-      // PIPELINING: Serialize pages WHILE previous fsync may still be running
-      // This overlaps CPU work (serialization) with IO work (fsync)
+      // PIPELINING: Serialize pages WHILE previous fsync may still be running.
+      // This overlaps CPU work (serialization) with IO work (fsync).
       parallelSerializationOfKeyValuePages();
 
-      // Recursively write indirectly referenced pages (serializes to buffers)
+      // Recursively write indirectly referenced pages (serializes to buffers).
       uberPage.commit(this);
 
-      // NOW wait for previous fsync before writing to storage
-      // This ensures ordering: previous commit is durable before new data hits disk
+      // Wait for the previous commit's async UberPage fsync to complete.
+      // This ensures the previous revision is fully durable before we proceed.
       if (pendingFsync != null) {
         pendingFsync.join();
+        pendingFsync = null;
       }
 
-      // Write pages to storage (previous fsync complete, safe to write)
+      // CRITICAL crash-safety invariant (write-ahead property):
+      // All data pages MUST be flushed to durable storage BEFORE the UberPage is written.
+      // If the process crashes between writing data pages and writing the UberPage, the OS
+      // kernel may flush the UberPage before the data pages, leaving the database pointing at
+      // pages that are not yet on disk.  An explicit forceAll() here prevents that window.
+      storagePageReaderWriter.forceAll();
+
+      // Write UberPage — all data pages are now durable, safe to update the root pointer.
       storagePageReaderWriter.writeUberPageReference(getResourceSession().getResourceConfig(), uberPageReference,
           uberPage, bufferBytes);
 
       if (isAutoCommitting) {
-        // Auto-commit mode: fsync runs asynchronously for better throughput
-        // Next commit's serialization will overlap with this fsync
+        // Auto-commit mode: queue an async fsync for the UberPage so the next commit's
+        // serialization can overlap with this IO.  The next commit will call pendingFsync.join()
+        // before writing its own UberPage, guaranteeing ordering.
+        // Even if the process crashes before this fsync completes the database is consistent:
+        // the old (pre-UberPage) state is recovered because the new UberPage is not yet on disk.
         pendingFsync = java.util.concurrent.CompletableFuture.runAsync(() -> {
           storagePageReaderWriter.forceAll();
         });
       } else {
-        // Regular commit: synchronous fsync for strict durability
-        // Data is guaranteed durable when commit() returns
+        // Regular commit: flush the UberPage synchronously so durability is guaranteed
+        // before commit() returns to the caller.
         storagePageReaderWriter.forceAll();
         pendingFsync = null;
       }
@@ -979,6 +990,12 @@ public void close() {
         pendingFsync = null;
       }
 
+      // Release the demotion buffer's off-heap memory.
+      if (demotionBuffer != null) {
+        demotionBuffer.close();
+        demotionBuffer = null;
+      }
+
       // Don't clear the cached containers here - they've either been:
       // 1. Already cleared and returned to pool during commit(), or
       // 2. Will be cleared and returned to pool by log.close() below

bundles/sirix-core/src/main/java/io/sirix/diff/algorithm/fmse/FMSE.java

@@ -191,11 +191,7 @@ public void diff(final XmlNodeTrx wtx, final XmlNodeReadOnlyTrx rtx) {
     final var fastMatching = fastMatch(this.wtx, this.rtx);
     totalMatching = new Matching(fastMatching);
     firstFMESStep(this.wtx, this.rtx);
-    try {
-      secondFMESStep(this.wtx, this.rtx);
-    } catch (final SirixException e) {
-      logger.error(e.getMessage(), e);
-    }
+    secondFMESStep(this.wtx, this.rtx);
   }
 
   /**
@@ -271,12 +267,8 @@ private void doFirstFSMEStep(final XmlNodeTrx wtx, final XmlNodeReadOnlyTrx rtx)
         rtx.moveTo(x);
         if (rtx.isNamespace() || rtx.isAttribute()) {
           wtx.moveTo(w);
-          try {
-            totalMatching.remove(w);
-            wtx.remove();
-          } catch (final SirixException e) {
-            logger.error(e.getMessage(), e);
-          }
+          totalMatching.remove(w);
+          wtx.remove();
           w = emitInsert(x, z, -1, wtx, rtx);
         } else {
           final int k = findPos(x, wtx, rtx);
@@ -446,8 +438,7 @@ private void emitMove(final long child, final long parent, final int pos, final
     moved = wtx.moveTo(parent);
     assert moved;
 
-    try {
-      if (pos == 0) {
+    if (pos == 0) {
         assert wtx.getKind() == NodeKind.ELEMENT || wtx.getKind() == NodeKind.XML_DOCUMENT;
         if (wtx.getFirstChildKey() == child) {
           logger.error("Something went wrong: First child and child may never be the same!");
@@ -512,9 +503,6 @@ private void emitMove(final long child, final long parent, final int pos, final
 
         wtx.moveTo(wtx.moveSubtreeToRightSibling(child).getNodeKey());
       }
-    } catch (final SirixException e) {
-      logger.error(e.getMessage(), e);
-    }
   }
 
   private void checkFromNodeForTextRemoval(final XmlNodeTrx wtx, final long child) {
@@ -562,26 +550,22 @@ private static void emitUpdate(final long fromNode, final long toNode, final Xml
     wtx.moveTo(fromNode);
     rtx.moveTo(toNode);
 
-    try {
-      switch (rtx.getKind()) {
-        case ELEMENT, ATTRIBUTE, NAMESPACE, PROCESSING_INSTRUCTION -> {
-          assert rtx.getKind() == NodeKind.ELEMENT || rtx.getKind() == NodeKind.ATTRIBUTE
-              || rtx.getKind() == NodeKind.NAMESPACE || rtx.getKind() == NodeKind.PROCESSING_INSTRUCTION;
-          wtx.setName(rtx.getName());
-          if (wtx.getKind() == NodeKind.ATTRIBUTE || wtx.getKind() == NodeKind.PROCESSING_INSTRUCTION) {
-            wtx.setValue(rtx.getValue());
-          }
-        }
-        case TEXT, COMMENT -> {
-          assert wtx.getKind() == NodeKind.TEXT;
+    switch (rtx.getKind()) {
+      case ELEMENT, ATTRIBUTE, NAMESPACE, PROCESSING_INSTRUCTION -> {
+        assert rtx.getKind() == NodeKind.ELEMENT || rtx.getKind() == NodeKind.ATTRIBUTE
+            || rtx.getKind() == NodeKind.NAMESPACE || rtx.getKind() == NodeKind.PROCESSING_INSTRUCTION;
+        wtx.setName(rtx.getName());
+        if (wtx.getKind() == NodeKind.ATTRIBUTE || wtx.getKind() == NodeKind.PROCESSING_INSTRUCTION) {
           wtx.setValue(rtx.getValue());
         }
-        // $CASES-OMITTED$
-        default -> {
-        }
       }
-    } catch (final SirixException e) {
-      logger.error(e.getMessage(), e);
+      case TEXT, COMMENT -> {
+        assert wtx.getKind() == NodeKind.TEXT;
+        wtx.setValue(rtx.getValue());
+      }
+      // $CASES-OMITTED$
+      default -> {
+      }
     }
   }
 
@@ -611,8 +595,7 @@ private long emitInsert(final long child, final long parent, final int pos, fina
     wtx.moveTo(parent);
     rtx.moveTo(child);
 
-    try {
-      switch (rtx.getKind()) {
+    switch (rtx.getKind()) {
         case ATTRIBUTE -> {
           try {
             wtx.insertAttribute(rtx.getName(), rtx.getValue());
@@ -726,12 +709,8 @@ private long emitInsert(final long child, final long parent, final int pos, fina
           }
         }
       }
-    } catch (final SirixException e) {
-      logger.error(e.getMessage(), e);
-    }
-
     return wtx.getNodeKey();
-  }
+    }
 
   /**
    * Remove right sibling text node from the storage as well as from the matching.

bundles/sirix-core/src/main/java/io/sirix/io/ram/RAMStorage.java

@@ -188,14 +188,22 @@ public Writer writeUberPageReference(final ResourceConfiguration resourceConfigu
 
     @Override
     public Instant readRevisionRootPageCommitTimestamp(int revision) {
-      // FIXME
-      throw new UnsupportedOperationException();
+      final RevisionRootPage revisionRootPage = mResourceRevisionRootsStorage.get(revision);
+      if (revisionRootPage == null) {
+        throw new SirixIOException("No revision root page found for revision " + revision
+            + " in RAM storage. RAMStorage does not persist revision root pages across restarts.");
+      }
+      return Instant.ofEpochMilli(revisionRootPage.getRevisionTimestamp());
     }
 
     @Override
     public RevisionFileData getRevisionFileData(int revision) {
-      // FIXME
-      throw new UnsupportedOperationException();
+      final RevisionRootPage revisionRootPage = mResourceRevisionRootsStorage.get(revision);
+      if (revisionRootPage == null) {
+        throw new SirixIOException("No revision root page found for revision " + revision
+            + " in RAM storage. RAMStorage does not persist revision root pages across restarts.");
+      }
+      return new RevisionFileData(0L, Instant.ofEpochMilli(revisionRootPage.getRevisionTimestamp()));
     }
 
     @Override

bundles/sirix-core/src/main/java/io/sirix/page/IndirectPage.java

@@ -58,16 +58,20 @@ public IndirectPage(final Page delegate) {
    * Clone indirect page.
    *
    * @param page {@link IndirectPage} to clone
+   * @throws IllegalStateException if the delegate type is unknown and cannot be copied
    */
   public IndirectPage(final IndirectPage page) {
     final Page pageDelegate = page.delegate();
 
-    if (pageDelegate instanceof ReferencesPage4) {
-      delegate = new ReferencesPage4((ReferencesPage4) pageDelegate);
-    } else if (pageDelegate instanceof BitmapReferencesPage) {
-      delegate = new BitmapReferencesPage(pageDelegate, ((BitmapReferencesPage) pageDelegate).getBitmap());
-    } else if (pageDelegate instanceof FullReferencesPage) {
-      delegate = new FullReferencesPage((FullReferencesPage) pageDelegate);
+    if (pageDelegate instanceof ReferencesPage4 ref) {
+      delegate = new ReferencesPage4(ref);
+    } else if (pageDelegate instanceof BitmapReferencesPage bmp) {
+      delegate = new BitmapReferencesPage(pageDelegate, bmp.getBitmap());
+    } else if (pageDelegate instanceof FullReferencesPage full) {
+      delegate = new FullReferencesPage(full);
+    } else {
+      throw new IllegalStateException(
+          "Unknown IndirectPage delegate type, cannot clone: " + pageDelegate.getClass().getName());
     }
   }
 

bundles/sirix-core/src/main/java/io/sirix/page/KeyValueLeafPage.java

@@ -673,7 +673,11 @@ public long getPageKey() {
   }
 
   @Override
-  public DataRecord getRecord(int offset) {
+  public DataRecord getRecord(final int offset) {
+    if (offset < 0 || offset >= Constants.NDP_NODE_COUNT) {
+      throw new IllegalArgumentException(
+          "Record offset out of range [0, " + Constants.NDP_NODE_COUNT + "): " + offset);
+    }
     return records[offset];
   }
 
@@ -685,8 +689,13 @@ public DataRecord getRecord(int offset) {
    * the cached record must be cleared to force re-materialization from those bytes on the next read.
    *
    * @param offset the slot offset to clear
+   * @throws IllegalArgumentException if offset is out of range [0, {@link Constants#NDP_NODE_COUNT})
    */
   public void clearRecord(final int offset) {
+    if (offset < 0 || offset >= Constants.NDP_NODE_COUNT) {
+      throw new IllegalArgumentException(
+          "Record offset out of range [0, " + Constants.NDP_NODE_COUNT + "): " + offset);
+    }
     if (records[offset] != null) {
       records[offset] = null;
       inMemoryRecordCount--;

bundles/sirix-core/src/main/java/io/sirix/page/PageReference.java

@@ -77,7 +77,8 @@ public PageReference() {
   }
 
   /**
-   * Copy constructor.
+   * Copy constructor. Creates an independent copy; the {@code pageFragments} list is deep-copied so
+   * mutations to the copy do not affect the original.
    *
    * @param reference {@link PageReference} to copy
    */
@@ -88,7 +89,9 @@ public PageReference(final PageReference reference) {
     databaseId = reference.databaseId;
     resourceId = reference.resourceId;
     hashInBytes = reference.hashInBytes;
-    pageFragments = reference.pageFragments;
+    pageFragments = reference.pageFragments != null
+        ? new ArrayList<>(reference.pageFragments)
+        : new ArrayList<>();
     hash = reference.hash;
   }