diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/DatasetGraphTDB.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/DatasetGraphTDB.java index 9f4b699ec30..bcbf353f819 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/DatasetGraphTDB.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/DatasetGraphTDB.java @@ -24,7 +24,6 @@ import org.apache.jena.atlas.iterator.Iter; import org.apache.jena.atlas.lib.tuple.Tuple; import org.apache.jena.dboe.base.file.Location; -import org.apache.jena.dboe.storage.StoragePrefixes; import org.apache.jena.dboe.storage.system.DatasetGraphStorage; import org.apache.jena.dboe.trans.bplustree.BPlusTree; import org.apache.jena.dboe.transaction.txn.TransactionalSystem; @@ -42,6 +41,7 @@ public class DatasetGraphTDB extends DatasetGraphStorage { private final StorageTDB storageTDB; + private final StoragePrefixesTDB storagePrefixes; private final Location location; private final TransactionalSystem txnSystem; private final StoreParams storeParams; @@ -49,8 +49,9 @@ public class DatasetGraphTDB extends DatasetGraphStorage private boolean isClosed = false; public DatasetGraphTDB(Location location, StoreParams params, ReorderTransformation reorderTransformation, - StorageTDB storage, StoragePrefixes prefixes, TransactionalSystem txnSystem) { + StorageTDB storage, StoragePrefixesTDB prefixes, TransactionalSystem txnSystem) { super(storage, prefixes, txnSystem); + this.storagePrefixes = prefixes; this.storageTDB = storage; this.location = location; this.storeParams = params; @@ -96,13 +97,17 @@ public ReorderTransformation getReorderTransform() { @Override public void close() { + if ( isClosed ) + return; isClosed = true; super.close(); + storageTDB.close(); + storagePrefixes.close(); } public void shutdown() { - close(); txnSystem.getTxnMgr().shutdown(); + close(); } @Override diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StoragePrefixesTDB.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StoragePrefixesTDB.java index 19991f9a4aa..bfe9693f0ac 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StoragePrefixesTDB.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StoragePrefixesTDB.java @@ -37,9 +37,10 @@ public class StoragePrefixesTDB implements StoragePrefixes { - static final RecordFactory factory = new RecordFactory(3*NodeId.SIZE, 0); + /*package*/ static final RecordFactory factory = new RecordFactory(3*NodeId.SIZE, 0); private TransactionalSystem txnSystem; private NodeTupleTable prefixTable; + private boolean closed = false; public StoragePrefixesTDB(TransactionalSystem txnSystem, NodeTupleTable prefixTable) { this.txnSystem = txnSystem; @@ -149,4 +150,14 @@ private void ensureWriteTxn() { throw new TransactionException("Not in a transaction"); txn.ensureWriteTxn(); } + + // This does not need to be synchronized. + // The caller is responsible for a quiet system (e.g. no transactions active) + /*package*/ void close() { + if ( closed ) + return; + closed = true; + prefixTable.close(); + } + } diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StorageTDB.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StorageTDB.java index 9da9a4e6849..7a006100d6c 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StorageTDB.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/StorageTDB.java @@ -33,11 +33,9 @@ /** {@link StorageRDF} for TDB2 */ public class StorageTDB implements StorageRDF { - // SWITCHING. This could be the switch point, not the DatasetGraph. Probably makes little difference. private TripleTable tripleTable; private QuadTable quadTable; private TransactionalSystem txnSystem; - // SWITCHING. // In notifyAdd and notifyDelete, check whether the change is a real change or not. // e.g. Adding a quad already present is not a real change. @@ -202,4 +200,14 @@ private void ensureWriteTxn() { throw new TransactionException("Not in a write transaction"); txn.ensureWriteTxn(); } + + // This does not need to be synchronized. + // The caller is responsible for a quiet system (e.g. no transactions active) + /*package*/ void close() { + if ( closed ) + return; + closed = true; + tripleTable.close(); + quadTable.close(); + } } diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/TDB2StorageBuilder.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/TDB2StorageBuilder.java index 4d2dd248859..b40249230ea 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/TDB2StorageBuilder.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/store/TDB2StorageBuilder.java @@ -31,7 +31,6 @@ import org.apache.jena.dboe.index.Index; import org.apache.jena.dboe.index.RangeIndex; import org.apache.jena.dboe.storage.DatabaseRDF; -import org.apache.jena.dboe.storage.StoragePrefixes; import org.apache.jena.dboe.sys.Names; import org.apache.jena.dboe.trans.bplustree.BPlusTree; import org.apache.jena.dboe.trans.bplustree.BPlusTreeFactory; @@ -95,7 +94,7 @@ public static DatasetGraphTDB build(Location location, StoreParams params, Reord TDB2StorageBuilder builder = new TDB2StorageBuilder(txnSystem, location, params, new ComponentIdMgr(UUID.randomUUID())); StorageTDB storage = builder.buildStorage(); - StoragePrefixes prefixes = builder.buildPrefixes(); + StoragePrefixesTDB prefixes = builder.buildPrefixes(); // Finalize. builder.components.forEach(txnCoord::add); @@ -193,7 +192,7 @@ private StorageTDB buildStorage() { return dsg; } - private StoragePrefixes buildPrefixes() { + private StoragePrefixesTDB buildPrefixes() { NodeTable nodeTablePrefixes = buildNodeTable(params.getPrefixTableBaseName(), false); StoragePrefixesTDB prefixes = buildPrefixTable(nodeTablePrefixes); return prefixes; diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOps.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOps.java index 429a6899f3e..96c79a41881 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOps.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOps.java @@ -311,7 +311,8 @@ public static void compact(DatasetGraphSwitchable container) { public static void compact(DatasetGraphSwitchable container, boolean shouldDeleteOld) { if ( Sys.isWindows) { // Windows does not support Files.move when the directory contains memory mapped files. - // https://github.com/apache/jena/issues/2315 + // MS Windows: 2024-03-08 https://github.com/apache/jena/issues/2315 + // Moving the temporary directory does not work. DatabaseOpsWindows.compact_win(container, shouldDeleteOld); return; } @@ -342,12 +343,11 @@ public static void compact(DatasetGraphSwitchable container, boolean shouldDelet LOG.debug(String.format("Compact %s -> %s\n", db1.getFileName(), db2.getFileName())); if ( Files.exists(db2) ) throw new TDBException("Inconsistent : "+db2+" already exists"); - - // MS Windows: 2024-03-08 https://github.com/apache/jena/issues/2315 - // Moving the temporary directory does not work. + // End checks // Location of the storage area for the compacted database. // This is a temporary directory that is atomically moved into place when complete. + // This is not supported by MS Windows. Path tmpDir = makeTempDirName(db2); if ( Files.exists(tmpDir) ) @@ -358,6 +358,7 @@ public static void compact(DatasetGraphSwitchable container, boolean shouldDelet try { compaction(container, loc1, loc2tmp, db2); // Container now using the new location. + // The original database is not in use. } catch (RuntimeIOException ex) { // Clear up - disk problems. try { IO.deleteAll(tmpDir); } catch (Throwable th) { /* Continue with original error. */ } @@ -473,7 +474,9 @@ private static void compaction(DatasetGraphSwitchable container, Location loc1, // This call is not undone. // Database1 is no longer in use. txnMgr1.startExclusiveMode(); + // Clean-up. + // Includes dsgBase.shutdown() which closes files. StoreConnection.release(dsgBase.getLocation()); } diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOpsWindows.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOpsWindows.java index def9c504a03..8ae23a58926 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOpsWindows.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/DatabaseOpsWindows.java @@ -35,7 +35,6 @@ import org.apache.jena.atlas.io.IO; import org.apache.jena.atlas.io.IOX; import org.apache.jena.atlas.lib.InternalErrorException; -import org.apache.jena.atlas.lib.Lib; import org.apache.jena.atlas.logging.FmtLog; import org.apache.jena.atlas.logging.Log; import org.apache.jena.dboe.DBOpEnvException; @@ -49,9 +48,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Old DatabaseOps code for compaction (up to Jena 5.0.0-rc1). - * Needed for MS Windows. - * +/** + * Old DatabaseOps code for compaction (up to Jena 5.0.0-rc1). + * Needed for MS Windows: + * (1) does not allow directories with memory -mapped file to be renamed/moved + * (2) does not release memory-mapped files until the JVM exits */ class DatabaseOpsWindows { private static Logger LOG = LoggerFactory.getLogger(DatabaseOpsWindows.class); @@ -101,8 +102,9 @@ class DatabaseOpsWindows { LOG.debug(String.format("Compact %s -> %s\n", db1.getFileName(), db2.getFileName())); try { - compaction(container, loc1, loc2); + compaction_win(container, loc1, loc2); // Container now using the new location. + // The original database is not in use. } catch (RuntimeIOException ex) { // Clear up - disk problems. try { IO.deleteAll(db2); } catch (Throwable th) { /* Continue with original error. */ } @@ -123,6 +125,7 @@ class DatabaseOpsWindows { if ( shouldDeleteOld ) { // Compact put each of the databases into exclusive mode to do the switchover. // There are no previous transactions on the old database at this point. + // MS Windows. Does not take effect until the JVM exits. Path loc1Path = IO_DB.asPath(loc1); LOG.warn("Database will not be fully deleted until after a server restart: (old db path='" + loc1Path + "')"); deleteDatabase(loc1Path); @@ -130,18 +133,8 @@ class DatabaseOpsWindows { } } - private static void deleteDatabase(Path locationPath) { - try (Stream walk = Files.walk(locationPath)){ - walk.sorted(Comparator.reverseOrder()) - .map(Path::toFile) - .forEach(File::delete); - } catch (IOException ex) { - throw IOX.exception(ex); - } - } - /** Copy the latest version from one location to another. */ - private static void compaction(DatasetGraphSwitchable container, Location loc1, Location loc2) { + private static void compaction_win(DatasetGraphSwitchable container, Location loc1, Location loc2) { if ( loc1.isMem() || loc2.isMem() ) throw new TDBException("Compact involves a memory location: "+loc1+" : "+loc2); @@ -180,13 +173,6 @@ private static void compaction(DatasetGraphSwitchable container, Location loc1, DatasetGraphTDB dsgCompact = StoreConnection.connectCreate(loc2).getDatasetGraphTDB(); CopyDSG.copy(dsgBase, dsgCompact); - if ( false ) { - // DEVELOMENT. FAke a long copy time in state copy. - System.err.println("-- Inside compact 1"); - Lib.sleep(3_000); - System.err.println("-- Inside compact 2"); - } - TransactionCoordinator txnMgr2 = dsgCompact.getTxnSystem().getTxnMgr(); // Update TransactionCoordinator and switch over. txnMgr2.execExclusive(()->{ @@ -213,7 +199,6 @@ private static void compaction(DatasetGraphSwitchable container, Location loc1, // New database running. // New transactions go to this database. // Old readers continue on db1. - }); @@ -222,10 +207,22 @@ private static void compaction(DatasetGraphSwitchable container, Location loc1, // This call is not undone. // Database1 is no longer in use. txnMgr1.startExclusiveMode(); + // Clean-up. + // MS Windows does not fully release memory mapped until the JVM exits. StoreConnection.release(dsgBase.getLocation()); } + private static void deleteDatabase(Path locationPath) { + try (Stream walk = Files.walk(locationPath)){ + walk.sorted(Comparator.reverseOrder()) + .map(Path::toFile) + .forEach(File::delete); + } catch (IOException ex) { + throw IOX.exception(ex); + } + } + /** Copy certain configuration files from {@code loc1} to {@code loc2}. */ private static void copyConfigFiles(Location loc1, Location loc2) { FileFilter copyFiles = (pathname)->{ @@ -302,7 +299,6 @@ private static List scanForDirByPatternX(Path directory, String namebase, return paths; } - private static Pattern numberPattern = Pattern.compile("[\\d]+"); /** Given a filename in "base-NNNN(-text)" format, return the value of NNNN */ private static int extractIndexX(String name, String namebase, String nameSep) { diff --git a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/StoreConnection.java b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/StoreConnection.java index 868fa6a702c..89ff6053cfd 100644 --- a/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/StoreConnection.java +++ b/jena-tdb2/src/main/java/org/apache/jena/tdb2/sys/StoreConnection.java @@ -134,10 +134,9 @@ public static synchronized void internalExpel(Location location, boolean force) // No transactions at this point // (or we don't care and are clearing up forcefully.) + // This closes open files. sConn.getDatasetGraphTDB().shutdown(); - // Done by DatasetGraphTDB() - //txnCoord.shutdown(); sConn.isValid = false; cache.remove(location);