[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user asfgit closed the pull request at: https://github.com/apache/jena/pull/458 ---
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/458#discussion_r209314854
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java ---
@@ -364,4 +369,31 @@ public static String uniqueFilename(String directory,
String base, String ext) {
return null ;
}
}
+
+/** Delete everything from a {@code Path} start point, including the
path itself.
+ * This function works on files or directories.
+ * This function does not follow symbolic links.
+ */
+public static void deleteAll(Path start) {
+// Walks down the tree and deletes directories on the way backup.
+try {
+Files.walkFileTree(start, new SimpleFileVisitor() {
--- End diff --
I'm not worried about it. I'm always a bit suspicious on anon inner classes
because of the occasional GC problems, but I think I'm letting my suspicions
run away from me.
---
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/458#discussion_r209313977
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java ---
@@ -364,4 +369,31 @@ public static String uniqueFilename(String directory,
String base, String ext) {
return null ;
}
}
+
+/** Delete everything from a {@code Path} start point, including the
path itself.
+ * This function works on files or directories.
+ * This function does not follow symbolic links.
+ */
+public static void deleteAll(Path start) {
+// Walks down the tree and deletes directories on the way backup.
+try {
+Files.walkFileTree(start, new SimpleFileVisitor() {
--- End diff --
Could do though I prefer to put the action code near to its usage, all in
the method. (It is also an idiom (e.g. stackoverflow) to inline the visitor.)
---
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/458#discussion_r209313461
--- Diff:
jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/mgt/ActionDatasets.java
---
@@ -288,28 +294,89 @@ protected void execDeleteItem(HttpAction action) {
if ( ! action.getDataAccessPointRegistry().isRegistered(name) )
ServletOps.errorNotFound("No such dataset registered: "+name);
+// This acts as a lock.
systemDSG.begin(ReadWrite.WRITE) ;
boolean committed = false ;
+
try {
// Here, go offline.
// Need to reference count operations when they drop to zero
// or a timer goes off, we delete the dataset.
DataAccessPoint ref =
action.getDataAccessPointRegistry().get(name) ;
+
// Redo check inside transaction.
if ( ref == null )
ServletOps.errorNotFound("No such dataset registered:
"+name);
+
+String filename = name.startsWith("/") ? name.substring(1) :
name;
+List configurationFiles =
FusekiSystem.existingConfigurationFile(filename);
+if ( configurationFiles.size() != 1 ) {
+// This should not happen.
+action.log.warn(format("[%d] There are %d configuration
files, not one.", action.id, configurationFiles.size()));
+ServletOps.errorOccurred(
+format(
+"There are %d configuration files, not one. Delete
not performed; clearup of the filesystem needed.",
+action.id, configurationFiles.size()));
+}
+
+String cfgPathname = configurationFiles.get(0);
+
+// Delete configuration file.
+// Once deleted, server restart will not have the database.
+FileOps.deleteSilent(cfgPathname);
-// Make it invisible to the outside.
+// Get before removing.
+DatasetGraph database = ref.getDataService().getDataset();
+// Make it invisible in this running server.
action.getDataAccessPointRegistry().remove(name);
-// Delete configuration file.
-// Should be only one, undo damage if multiple.
-String filename = name.startsWith("/") ? name.substring(1) :
name;
-
FusekiSystem.existingConfigurationFile(filename).stream().forEach(FileOps::deleteSilent);
-// Leave the database in place. if it is in /databases/,
recreating the
-// configuration will make the database reappear. This is
intentional.
-// Deleting a large database by accident is a major mistake.
+// JENA-1481 & JENA-1586 : Delete the database.
+// Delete the database for real only when it is in the server
"run/databases"
+// area. Don't delete databases that reside elsewhere. We do
delete the
+// configuration file, so the databases will not be associated
with the server
+// anymore.
+
+boolean tryToRemoveFiles = true;
+
+// Text databases.
+// Close the in-JVM objects for Lucene index and databases.
+// Do not delete files; at least for the lucene index, they
are likely outside the run/databases.
+if ( database instanceof DatasetGraphText ) {
+DatasetGraphText dbtext = (DatasetGraphText)database;
+database = dbtext.getBase();
+dbtext.getTextIndex().close();
+tryToRemoveFiles = false ;
+}
+
+boolean isTDB1 =
org.apache.jena.tdb.sys.TDBInternal.isTDB1(database);
+boolean isTDB2 =
org.apache.jena.tdb2.sys.TDBInternal.isTDB2(database);
+
+if ( ( isTDB1 || isTDB2 ) ) {
+// JENA-1586: Remove database from the process.
+if ( isTDB1 )
+org.apache.jena.tdb.sys.TDBInternal.expel(database);
+if ( isTDB2 )
+org.apache.jena.tdb2.sys.TDBInternal.expel(database);
+
+// JENA-1481: Really delete files.
+// Find the database files (may not be any - e.g.
in-memory).
+Path pDatabase =
FusekiSystem.dirDatabases.resolve(filename);
+if ( tryToRemoveFiles && Files.exists(pDatabase)) {
+try {
+if ( Files.isSymbolicLink(pDatabase)) {
+action.log.info(format("[%d] D
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/458#discussion_r209309616
--- Diff:
jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/mgt/ActionDatasets.java
---
@@ -288,28 +294,89 @@ protected void execDeleteItem(HttpAction action) {
if ( ! action.getDataAccessPointRegistry().isRegistered(name) )
ServletOps.errorNotFound("No such dataset registered: "+name);
+// This acts as a lock.
systemDSG.begin(ReadWrite.WRITE) ;
boolean committed = false ;
+
try {
// Here, go offline.
// Need to reference count operations when they drop to zero
// or a timer goes off, we delete the dataset.
DataAccessPoint ref =
action.getDataAccessPointRegistry().get(name) ;
+
// Redo check inside transaction.
if ( ref == null )
ServletOps.errorNotFound("No such dataset registered:
"+name);
+
+String filename = name.startsWith("/") ? name.substring(1) :
name;
+List configurationFiles =
FusekiSystem.existingConfigurationFile(filename);
+if ( configurationFiles.size() != 1 ) {
+// This should not happen.
+action.log.warn(format("[%d] There are %d configuration
files, not one.", action.id, configurationFiles.size()));
+ServletOps.errorOccurred(
+format(
+"There are %d configuration files, not one. Delete
not performed; clearup of the filesystem needed.",
+action.id, configurationFiles.size()));
+}
+
+String cfgPathname = configurationFiles.get(0);
+
+// Delete configuration file.
+// Once deleted, server restart will not have the database.
+FileOps.deleteSilent(cfgPathname);
-// Make it invisible to the outside.
+// Get before removing.
+DatasetGraph database = ref.getDataService().getDataset();
+// Make it invisible in this running server.
action.getDataAccessPointRegistry().remove(name);
-// Delete configuration file.
-// Should be only one, undo damage if multiple.
-String filename = name.startsWith("/") ? name.substring(1) :
name;
-
FusekiSystem.existingConfigurationFile(filename).stream().forEach(FileOps::deleteSilent);
-// Leave the database in place. if it is in /databases/,
recreating the
-// configuration will make the database reappear. This is
intentional.
-// Deleting a large database by accident is a major mistake.
+// JENA-1481 & JENA-1586 : Delete the database.
+// Delete the database for real only when it is in the server
"run/databases"
+// area. Don't delete databases that reside elsewhere. We do
delete the
+// configuration file, so the databases will not be associated
with the server
+// anymore.
+
+boolean tryToRemoveFiles = true;
+
+// Text databases.
+// Close the in-JVM objects for Lucene index and databases.
+// Do not delete files; at least for the lucene index, they
are likely outside the run/databases.
+if ( database instanceof DatasetGraphText ) {
+DatasetGraphText dbtext = (DatasetGraphText)database;
+database = dbtext.getBase();
+dbtext.getTextIndex().close();
+tryToRemoveFiles = false ;
+}
+
+boolean isTDB1 =
org.apache.jena.tdb.sys.TDBInternal.isTDB1(database);
+boolean isTDB2 =
org.apache.jena.tdb2.sys.TDBInternal.isTDB2(database);
+
+if ( ( isTDB1 || isTDB2 ) ) {
+// JENA-1586: Remove database from the process.
+if ( isTDB1 )
+org.apache.jena.tdb.sys.TDBInternal.expel(database);
+if ( isTDB2 )
+org.apache.jena.tdb2.sys.TDBInternal.expel(database);
+
+// JENA-1481: Really delete files.
+// Find the database files (may not be any - e.g.
in-memory).
+Path pDatabase =
FusekiSystem.dirDatabases.resolve(filename);
+if ( tryToRemoveFiles && Files.exists(pDatabase)) {
+try {
+if ( Files.isSymbolicLink(pDatabase)) {
+action.log.info(format("[%d]
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/458#discussion_r209307965
--- Diff: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java ---
@@ -364,4 +369,31 @@ public static String uniqueFilename(String directory,
String base, String ext) {
return null ;
}
}
+
+/** Delete everything from a {@code Path} start point, including the
path itself.
+ * This function works on files or directories.
+ * This function does not follow symbolic links.
+ */
+public static void deleteAll(Path start) {
+// Walks down the tree and deletes directories on the way backup.
+try {
+Files.walkFileTree(start, new SimpleFileVisitor() {
--- End diff --
Could this `new SimpleFileVisitor` be broken out as a `static` inner class?
---
[GitHub] jena pull request #458: JENA-1481, JENA-1586: Delete databases and expel fro...
GitHub user afs opened a pull request: https://github.com/apache/jena/pull/458 JENA-1481, JENA-1586: Delete databases and expel from JVM You can merge this pull request into a Git repository by running: $ git pull https://github.com/afs/jena fuseki-db-delete Alternatively you can review and apply these changes as the patch at: https://github.com/apache/jena/pull/458.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #458 commit f5197905d57eee6500b7d4ed6cd87f2bd9ae738e Author: Andy Seaborne Date: 2018-08-10T15:21:39Z JENA-1481, JENA-1586: Delete databases and expel from JVM ---
