db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From krist...@apache.org
Subject svn commit: r883297 - in /db/derby/code/trunk/java/engine/org/apache/derby/impl/io: VFMemoryStorageFactory.java vfmem/DataStore.java
Date Mon, 23 Nov 2009 10:30:30 GMT
Author: kristwaa
Date: Mon Nov 23 10:30:30 2009
New Revision: 883297

URL: http://svn.apache.org/viewvc?rev=883297&view=rev
Log:
DERBY-4428 (partial): Add proper delete mechanism for in-memory databases
Added the code required in the in-memory back end storage factory.
Some logic is required to remove the data store from the list of databases when
a database is dropped. Note that this logic isn't fully general purpose - it is
partly dependent on how the monitor / StorageFactoryService works.

Patch file: derby-4428-1b-in_memory_specific_delete_code.diff


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/io/VFMemoryStorageFactory.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/io/vfmem/DataStore.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/io/VFMemoryStorageFactory.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/io/VFMemoryStorageFactory.java?rev=883297&r1=883296&r2=883297&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/io/VFMemoryStorageFactory.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/io/VFMemoryStorageFactory.java Mon
Nov 23 10:30:30 2009
@@ -38,6 +38,16 @@
 /**
  * A storage factory for virtual files, where the contents of the files are
  * stored in main memory.
+ * <p>
+ * Note that data store deletion may happen inside one of two different methods;
+ * either in {@code shutdown} or in {@code init}. This is due to the current
+ * implementation and the fact that dropping a database is done through the
+ * file IO interface by deleting the service root. As the deletion then becomes
+ * a two step process, someone else may boot the database again before the
+ * reference to the store has been removed. To avoid this, the
+ * {@code init}-method will never initialize with a store scheduled for
+ * deletion. I have only seen this issue in heavily loaded multithreaded
+ * environments (2 CPUs/cores should be enough to reproduce).
  */
 public class VFMemoryStorageFactory
         implements StorageFactory, WritableStorageFactory {
@@ -54,6 +64,7 @@
     private static final DataStore DUMMY_STORE = new DataStore("::DUMMY::");
 
     /**
+     * TODO: Remove this method once the new mechanism has been added.
      * Deletes the database if it exists.
      *
      * @param dbName the database name
@@ -118,19 +129,26 @@
                 canonicalName = new File(databaseName).getCanonicalPath();
             }
             synchronized (DATABASES) {
-                if (DATABASES.containsKey(canonicalName)) {
-                    // Fetch the existing data store.
-                    this.dbData = (DataStore)DATABASES.get(canonicalName);
-                } else if (uniqueName != null) {
-                    // Create a new data store.
-                    this.dbData = new DataStore(canonicalName);
-                    DATABASES.put(canonicalName, dbData);
-                } else {
-                    // We have a database name, but no unique name.
-                    // Assume that the client only wants to do some
-                    // "book-keeping" operations, like getting the
-                    // canonical name.
-                    this.dbData = DUMMY_STORE;
+                this.dbData = (DataStore)DATABASES.get(canonicalName);
+                // If the store has been scheduled for deletion, purge it.
+                if (dbData != null && dbData.scheduledForDeletion()) {
+                    DATABASES.remove(canonicalName);
+                    dbData.purge();
+                    dbDropCleanupInDummy(canonicalName);
+                    dbData = null;
+                }
+                if (dbData == null) {
+                    if (uniqueName != null) {
+                        // Create a new data store.
+                        this.dbData = new DataStore(canonicalName);
+                        DATABASES.put(canonicalName, dbData);
+                    } else {
+                        // We have a database name, but no unique name.
+                        // Assume that the client only wants to do some
+                        // "book-keeping" operations, like getting the
+                        // canonical name.
+                        this.dbData = DUMMY_STORE;
+                    }
                 }
             }
             // Specify the data directory and the temp directory.
@@ -161,11 +179,29 @@
         }
     }
 
+    /**
+     * Normally does nothing, but if the database is in a state such that it
+     * should be deleted this will happen here.
+     */
     public void shutdown() {
-        // For now, do nothing.
-        // TODO: Deleting stuff doesn't seem to play nice when running the
-        // regression tests, as CleanDatabaseTestSetup fails then. The cause
-        // is unknown and should be investigated.
+        // If the data store has been scheduled for deletion, which happens
+        // when the store detects that the service root has been deleted, then
+        // delete the whole store to release the memory.
+        if (dbData.scheduledForDeletion()) {
+            DataStore store;
+            synchronized (DATABASES) {
+                store = (DataStore)DATABASES.remove(canonicalName);
+                // Must clean up the dummy while holding monitor.
+                if (store != null && store == dbData) {
+                    dbDropCleanupInDummy(canonicalName);
+                }
+            }
+            // If this is the correct store, purge it now.
+            if (store != null && store == dbData) {
+                dbData.purge(); // Idempotent.
+                dbData = null;
+            }
+        }
     }
 
     public String getCanonicalName() {
@@ -336,4 +372,16 @@
             return new File(dataDirectory.getPath(), path).getPath();
         }
     }
+
+    /**
+     * Cleans up the internal dummy data store after a database has been
+     * dropped.
+     *
+     * @param dbPath absolute path of the dropped database
+     */
+    private void dbDropCleanupInDummy(String dbPath) {
+        while (dbPath != null && DUMMY_STORE.deleteEntry(dbPath)) {
+            dbPath = new File(dbPath).getParent();
+        }
+    }
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/io/vfmem/DataStore.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/io/vfmem/DataStore.java?rev=883297&r1=883296&r2=883297&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/io/vfmem/DataStore.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/io/vfmem/DataStore.java Mon Nov
23 10:30:30 2009
@@ -56,22 +56,30 @@
      */
     private final Map files = new HashMap(80);
 
-    /** The name of the database this store serves. */
+    /**
+     * The name of the database this store serves, expected to be the absolute
+     * path of the service root (i.e. /tmp/myDB if the database myDB is created
+     * in /tmp).
+     */
     private final String databaseName;
     /** Counter used to generate unique temporary file names. */
     private long tmpFileCounter = 0;
+    /** Tells if this store is scheduled for deletion. */
+    private boolean deleteMe;
 
     /**
      * Creates a new data store.
      *
-     * @param databaseName the name of the assoicated database
+     * @param databaseName the name of the assoicated database, expected to be
+     *      the absolute path of the service root.
      */
     public DataStore(String databaseName) {
         this.databaseName = databaseName;
     }
 
     /**
-     * Returns the database name.
+     * Returns the database name, which is expected to equal the path of the
+     * service root.
      *
      * @return The database name.
      */
@@ -80,6 +88,16 @@
     }
 
     /**
+     * Tells if this data store is scheduled for deletion.
+     *
+     * @return {@code true} if the store is awaiting deletion,
+     *      {@code false} otherwise.
+     */
+    public boolean scheduledForDeletion() {
+        return this.deleteMe;
+    }
+
+    /**
      * Creates a new entry in the data store.
      * <p>
      * This method returns {@code null} if the path already exists, if one of
@@ -161,6 +179,13 @@
                         files.put(nPath, entry);
                         return false;
                     }
+                    // Check if we just deleted the service root. Normally the
+                    // service would be deleted using deleteAll.
+                    if (nPath.equals(databaseName) &&
+                            files.get(databaseName) == null) {
+                        // Service root deleted, mark this store for removal.
+                        deleteMe = true;
+                    }
                 }
                 entry.release();
             }
@@ -198,7 +223,13 @@
                 return false;
             } else if (entry.isDirectory()) {
                 // Delete root is a directory.
-                return _deleteAll(nPath);
+                boolean deleted = _deleteAll(nPath);
+                if (files.get(databaseName) == null) {
+                    // The service root has been deleted, which means that all
+                    // the data has been deleted. Mark this store for removal.
+                    deleteMe = true;
+                }
+                return deleted;
             } else {
                 // Delete root is a file.
                 entry.release();



Mime
View raw message