Merge lp:~zeitgeist/zeitgeist/dbfails into lp:~zeitgeist/zeitgeist/bluebird

[Siegfried Gevatter]

[Siegfried Gevatter (rainct)]

<mhr3> RainCT, can you propagate the error one more level up, so there's no need for the posix.exit?

review: Needs Fixing

[lp:~zeitgeist/zeitgeist/dbfails]

357. By Michal Hruby on 2011-12-31

Add a define to explain query plans

358. By Michal Hruby on 2011-12-31

Merge lp:~zeitgeist/zeitgeist/bluebird-no-distinct

359. By Seif Lotfy on 2011-12-31

Add Exclusive lock for secure writing

360. By Michal Hruby on 2011-12-31

Add real-world query samples

361. By Michal Hruby on 2012-01-02

Don't use fts when using in-memory database

362. By Michal Hruby on 2012-01-02

Don't display debug messages by default

363. By Michal Hruby on 2012-01-02

Add a way to disable extensions

364. By Seif Lotfy on 2012-01-03

Fix bug 909708 where inserting and event with 2 subjects.
Since we already parse the subjects before insertion I added a list with subj_uris to find if there is a duplicate and return 0 if one is found

365. By Michal Hruby on 2012-01-06

Fix Reindex in FTS indexer

366. By Michal Hruby on 2012-01-06

Disable the VolumeMonitor in StorageMonitor extension to avoid weird races

[Michal Hruby (mhr3)]

Yes, please :)

review: Needs Fixing

[lp:~zeitgeist/zeitgeist/dbfails]

367. By Seif Lotfy on 2012-01-15

optimize allocations of variants

368. By Seif Lotfy on 2012-01-16

Seperate the allocation optimization into a new method optimize_variant_allocation.
This optimization effects marshalling by slowint it down by around 0.06s for 10k
events yet saves us 10MB in memory consumption.

369. By Siegfried Gevatter on 2012-01-25

Merge Seif's FTS changes removing database writes.

370. By Siegfried Gevatter on 2012-01-25

Automatically recover from corrupt database and be more clear
on some other errors.

[Michal Hruby (mhr3)]

282 + throw err;
289 + throw err;
293 + throw err;

Seems like it could be moved out of the ifs.

Other than that it's fine.

review: Approve

=== modified file 'doc/zeitgeist-daemon.1'
--- doc/zeitgeist-daemon.1	2012-01-02 19:31:15 +0000
+++ doc/zeitgeist-daemon.1	2012-01-25 11:17:27 +0000
@@ -87,6 +87,12 @@
 .TP
 .B 10
 There is already a running Zeitgeist instance.
+.TP
+.B 21
+Could not access the database file.
+.TP
+.B 22
+The database file is locked.
 
 .SH SEE ALSO
 \fBzeitgeist-datahub\fR, \fBgnome-activity-journal\fR

=== modified file 'src/errors.vala'
--- src/errors.vala	2011-10-20 13:32:51 +0000
+++ src/errors.vala	2012-01-25 11:17:27 +0000
@@ -23,10 +23,15 @@
     [DBus (name = "org.gnome.zeitgeist.EngineError")]
     public errordomain EngineError
     {
+        BACKUP_FAILED,
+        DATABASE_BUSY,
+        DATABASE_CANTOPEN,
+        DATABASE_CORRUPT,
         DATABASE_ERROR,
+        DATABASE_RETIRE_FAILED,
         INVALID_ARGUMENT,
         INVALID_KEY,
-        BACKUP_FAILED,
+        EXISTING_INSTANCE,
     }
 
     // vala doesn't include proper headers, this fixes it

=== modified file 'src/sql-schema.vala'
--- src/sql-schema.vala	2011-12-31 19:15:54 +0000
+++ src/sql-schema.vala	2012-01-25 11:17:27 +0000
@@ -414,8 +414,8 @@
         }
 
         /**
-         * Execute the given SQL. If the query doesn't succeed, log a
-         * critical warning (potentially aborting the program).
+         * Execute the given SQL. If the query doesn't succeed, throw
+         * an error.
          *
          * @param database the database on which to run the query
          * @param sql the SQL query to run
@@ -426,10 +426,17 @@
             int rc = database.exec (sql);
             if (rc != Sqlite.OK)
             {
-                const string fmt_str = "Can't create database: %d, %s\n\n" +
-                    "Unable to execute SQL:\n%s";
-                var err_msg = fmt_str.printf (rc, database.errmsg (), sql);
-                throw new EngineError.DATABASE_ERROR (err_msg);
+                if (rc == Sqlite.CORRUPT)
+                {
+                    throw new EngineError.DATABASE_CORRUPT (database.errmsg ());
+                }
+                else
+                {
+                    const string fmt_str = "Can't create database: %d, %s\n\n" +
+                        "Unable to execute SQL:\n%s";
+                    var err_msg = fmt_str.printf (rc, database.errmsg (), sql);
+                    throw new EngineError.DATABASE_ERROR (err_msg);
+                }
             }
         }
 

=== modified file 'src/sql.vala'
--- src/sql.vala	2012-01-02 19:30:51 +0000
+++ src/sql.vala	2012-01-25 11:17:27 +0000
@@ -67,13 +67,7 @@
 
         public ZeitgeistDatabase () throws EngineError
         {
-            message ("Opening DB from %s", Utils.get_database_file_path ());
-            int rc = Sqlite.Database.open_v2 (
-                Utils.get_database_file_path (),
-                out database);
-            assert_query_success (rc, "Can't open database");
-
-            DatabaseSchema.ensure_schema (database);
+            open_database (true);
 
             prepare_queries ();
 
@@ -82,6 +76,74 @@
             database.update_hook (update_callback);
         }
 
+        private void open_database (bool retry)
+            throws EngineError
+        {
+            int rc = Sqlite.Database.open_v2 (
+                Utils.get_database_file_path (),
+                out database);
+            
+            if (rc == Sqlite.OK)
+            {
+                try
+                {
+                    // Error (like a malformed database) may not be exposed
+                    // until we try to operate on the database.
+                    DatabaseSchema.ensure_schema (database);
+                }
+                catch (EngineError err)
+                {
+                    if (err is EngineError.DATABASE_CORRUPT && retry)
+                        rc = Sqlite.CORRUPT;
+                    else if (err is EngineError.DATABASE_CANTOPEN)
+                        rc = Sqlite.CANTOPEN;
+                    else if (err is EngineError.DATABASE_BUSY)
+                        rc = Sqlite.BUSY;
+                    else
+                        throw err;
+                }
+            }
+            
+            if (rc != Sqlite.OK)
+            {
+                if (rc == Sqlite.CORRUPT && retry)
+                {
+                    // The database disk image is malformed
+                    warning ("It looks like your database is corrupt. " +
+                        "It will be renamed and a new one will be created.");
+                    try
+                    {
+                        Utils.retire_database ();
+                    }
+                    catch (Error err)
+                    {
+                        string message =
+                            "Could not rename database: %s".printf (
+                                err.message);
+                        throw new EngineError.DATABASE_RETIRE_FAILED (message);
+                    }
+                    open_database (false);
+                }
+                else if (rc == Sqlite.PERM || rc == Sqlite.CANTOPEN)
+                {
+                    // Access permission denied / Unable to open database file
+                    throw new EngineError.DATABASE_CANTOPEN (
+                        database.errmsg ());
+                }
+                else if (rc == Sqlite.BUSY)
+                {
+                    // The database file is locked
+                    throw new EngineError.DATABASE_BUSY (database.errmsg ());
+                }
+                else
+                {
+                    string message = "Can't open database: %d, %s".printf(rc,
+                        database.errmsg ());
+                    throw new EngineError.DATABASE_ERROR (message);
+                }
+            }
+        }
+
         public uint32 get_last_id () throws EngineError
         {
             int last_id = -1;

=== modified file 'src/utils.vala'
--- src/utils.vala	2011-10-31 15:28:09 +0000
+++ src/utils.vala	2012-01-25 11:17:27 +0000
@@ -31,7 +31,8 @@
         private static string DATABASE_FILE_BACKUP_PATH;
         private static string LOCAL_EXTENSIONS_PATH;
 
-        public const string ZEITGEIST_DATA_FOLDER = "zeitgeist";
+        public const string DATA_FOLDER = "zeitgeist";
+        public const string DATABASE_BASENAME = "activity.sqlite";
         public const string USER_EXTENSION_PATH = "";
 
         // D-Bus
@@ -48,7 +49,7 @@
 
             DATA_PATH = Environment.get_variable ("ZEITGEIST_DATA_PATH") ??
                 Path.build_filename (Environment.get_user_data_dir (),
-                    ZEITGEIST_DATA_FOLDER);
+                    DATA_FOLDER);
 
             if (!FileUtils.test (DATA_PATH, FileTest.IS_DIR))
             {
@@ -66,7 +67,7 @@
 
             DATABASE_FILE_PATH =
                 Environment.get_variable ("ZEITGEIST_DATABASE_PATH") ??
-                Path.build_filename (get_data_path (), "activity.sqlite");
+                Path.build_filename (get_data_path (), DATABASE_BASENAME);
 
             debug ("DATABASE_FILE_PATH = %s", DATABASE_FILE_PATH);
 
@@ -80,13 +81,20 @@
 
             DATABASE_FILE_BACKUP_PATH =
                 Environment.get_variable ("ZEITGEIST_DATABASE_BACKUP_PATH") ??
-                Path.build_filename (get_data_path (), "activity.sqlite.bck");
+                Path.build_filename (get_data_path (),
+                    DATABASE_BASENAME + ".bck");
 
             debug ("DATABASE_FILE_BACKUP_PATH = %s", DATABASE_FILE_BACKUP_PATH);
 
             return DATABASE_FILE_BACKUP_PATH;
         }
 
+        public string get_database_file_retire_name ()
+        {
+            return DATABASE_BASENAME + ".%s.bck".printf (
+                new DateTime.now_local ().format ("%Y%m%d-%H%M%S"));
+        }
+
         public unowned string get_local_extensions_path ()
         {
             if (LOCAL_EXTENSIONS_PATH != null) return LOCAL_EXTENSIONS_PATH;
@@ -113,6 +121,12 @@
 
             original.copy (destination, FileCopyFlags.OVERWRITE, null, null);
         }
+
+        public void retire_database () throws Error
+        {
+            File dbfile = File.new_for_path (get_database_file_path ());
+            dbfile.set_display_name (get_database_file_retire_name ());
+        }
     }
 }
 

=== modified file 'src/zeitgeist-daemon.vala'
--- src/zeitgeist-daemon.vala	2012-01-02 19:30:51 +0000
+++ src/zeitgeist-daemon.vala	2012-01-25 11:17:27 +0000
@@ -329,6 +329,7 @@
         }
 
         static void run ()
+            throws Error
         {
             DBusConnection connection;
             bool name_owned;
@@ -342,8 +343,7 @@
             }
             catch (IOError err)
             {
-                critical ("%s", err.message);
-                return;
+                throw err;
             }
             if (name_owned)
             {
@@ -353,9 +353,10 @@
                 }
                 else
                 {
-                    critical ("An existing instance was found. Please use " +
+                    warning ("An existing instance was found. Please use " +
                         "--replace to stop it and start a new instance.");
-                    Posix.exit (10);
+                    throw new EngineError.EXISTING_INSTANCE (
+                        "Zeitgeist is running already.");
                 }
             }
 
@@ -370,8 +371,24 @@
             }
             catch (Error err)
             {
-                critical ("%s", err.message);
-                return;
+                if (err is EngineError.DATABASE_CANTOPEN)
+                {
+                    warning ("Could not access the database file.\n" +
+                        "Please check the permissions of file %s.",
+                        Utils.get_database_file_path ());
+                    throw err;
+                }
+                else if (err is EngineError.DATABASE_BUSY)
+                {
+                    warning ("It looks like another Zeitgeist instance " +
+                        "is already running (the database is locked). " +
+                        "If you want to start a new instance, use --replace.");
+                    throw err;
+                }
+                else
+                {
+                    throw err;
+                }
             }
 
             uint owner_id = Bus.own_name_on_connection (connection,
@@ -437,7 +454,7 @@
 
                     return 0;
                 }
-                
+
                 LogLevelFlags discarded = LogLevelFlags.LEVEL_DEBUG;
                 if (log_level != null)
                 {
@@ -472,9 +489,22 @@
 
                 run ();
             }
+            catch (EngineError.EXISTING_INSTANCE err)
+            {
+                return 10;
+            }
+            catch (EngineError.DATABASE_CANTOPEN err)
+            {
+                return 21;
+            }
+            catch (EngineError.DATABASE_BUSY err)
+            {
+                return 22;
+            }
             catch (Error err)
             {
                 warning ("%s", err.message);
+                return 1;
             }
 
             return 0;