This is an automated email from the ASF dual-hosted git repository.
andy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/jena.git
The following commit(s) were added to refs/heads/main by this push:
new a3983b2a6d GH-3015: Close files after TDB2 compaction
a3983b2a6d is described below
commit a3983b2a6d26c5dc7d849b7d5f2f550f9d607eb3
Author: Andy Seaborne <[email protected]>
AuthorDate: Fri Feb 21 11:54:00 2025 +0000
GH-3015: Close files after TDB2 compaction
---
.../apache/jena/tdb2/store/DatasetGraphTDB.java | 11 ++++--
.../apache/jena/tdb2/store/StoragePrefixesTDB.java | 13 +++++-
.../org/apache/jena/tdb2/store/StorageTDB.java | 12 +++++-
.../apache/jena/tdb2/store/TDB2StorageBuilder.java | 5 +--
.../java/org/apache/jena/tdb2/sys/DatabaseOps.java | 11 ++++--
.../apache/jena/tdb2/sys/DatabaseOpsWindows.java | 46 ++++++++++------------
.../org/apache/jena/tdb2/sys/StoreConnection.java | 3 +-
7 files changed, 61 insertions(+), 40 deletions(-)
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 9f4b699ec3..bcbf353f81 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.commons.lang3.StringUtils;
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 @@ final
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 class DatasetGraphTDB extends DatasetGraphStorage
@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 19991f9a4a..bfe9693f0a 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 @@ import
org.apache.jena.tdb2.store.nodetupletable.NodeTupleTable;
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 @@ public class StoragePrefixesTDB implements StoragePrefixes
{
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 9da9a4e684..7a006100d6 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 @@ import org.apache.jena.sparql.core.Quad;
/** {@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 @@ public class StorageTDB implements StorageRDF {
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 4d2dd24885..b40249230e 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.base.record.RecordFactory;
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 class TDB2StorageBuilder {
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 @@ public class TDB2StorageBuilder {
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 429a6899f3..96c79a4188 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 class DatabaseOps {
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 class DatabaseOps {
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 class DatabaseOps {
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 @@ public class DatabaseOps {
// 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 def9c504a0..8ae23a5892 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.RuntimeIOException;
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.apache.jena.tdb2.store.DatasetGraphTDB;
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<Path> 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 @@ class DatabaseOpsWindows {
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 @@ class DatabaseOpsWindows {
// New database running.
// New transactions go to this database.
// Old readers continue on db1.
-
});
@@ -222,10 +207,22 @@ class DatabaseOpsWindows {
// 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<Path> 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 @@ class DatabaseOpsWindows {
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 868fa6a702..89ff6053cf 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 class StoreConnection
// 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);
